Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port Template DAO to use PDO #534

Merged
merged 6 commits into from
Nov 25, 2021
Merged

Port Template DAO to use PDO #534

merged 6 commits into from
Nov 25, 2021

Conversation

jaragunde
Copy link
Member

First steps towards adopting PDO (#513), starting with an isolated data type that's not used a lot.

This begins the work to replace pg_* functions for database access
with the modern PDO interface.

We first migrate PostgreSQLTemplateDAO::GetById and GetByUserId
methods. We will progressively extend PDO usage to the rest of DAOs,
refactoring code as needed along the way.
Refactor common code in PostgreSQLTemplateDAO::GetById and GetByUserId
to the function runSelectQuery.

This leaves those functions looking similar to what they looked like
before, when they used the execute function from the non-PDO code
paths.
Rewrite method to use PDO prepared statements, typed binding and last
id retrieval.
The check that looked up the template in the database before removing
it was deleted because it was useless. The query will run, returning
zero affected rows, if the template doesn't exist. From the outside,
there is no change of behavior in the delete operation.
The missing PDO package in Debian is not a typo, it seems to be
provided by another package.

I'm removing the RHEL line because I don't have a RHEL system where
I can test things, and I don't want to provide unverified commands.
We need to modify the properties in the VO class to match column names.
Besides, the TemplateVO::setValues() function cannot be removed because
it's declared abstract in a parent class. This should be temporary,
until all DAOs use PDO.
Copy link
Member

@anarute anarute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me and I also tested locally. Thanks for this improvement!

@jaragunde jaragunde merged commit 09a5efa into main Nov 25, 2021
@jaragunde jaragunde deleted the i513-pdo-for-template-table branch January 26, 2022 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants