-
Notifications
You must be signed in to change notification settings - Fork 175
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
[Core-Database] added 2 new functions to help avoid code duplication #3428
[Core-Database] added 2 new functions to help avoid code duplication #3428
Conversation
@HenriRabalais this might be useful for you |
php/libraries/Database.class.inc
Outdated
@@ -744,6 +744,77 @@ class Database | |||
return $result; | |||
} | |||
|
|||
/** | |||
* Runs an SQL select statement as a prepared query and re-indexes | |||
* the results using the given primary key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it needs to be a unique non-null column, not necessarily a primary key (which also affects the function name..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah your correct I had primary key everywhere before then changed it all except here I guess
@driusan fixed |
@ridz1208 The function names still reference "primary" |
@driusan yeah i know, not sure what to call them. And not sure if the exception messages are clear enough. Suggestion ? |
Some ideas, in no particular order:
|
@HenriRabalais Are you reviewing/testing this? |
|
||
return $filteredResult; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and will be very useful - this will replace a lot of the work that the biobank DAO is currently doing when querying the database.
|
||
return $filteredResult; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fail to see how this is useful in light of the first function - is the plan to make 1:1 index to value mappings? If that is the case, it does not seem that you are able to specify which column you are targeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I think of it, you're right. Doesn't pselectCol always return a single column? Isn't this just requiring an extra parameter @ridz1208?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here...
Imagine the following table (notice that the primary key is an autoincrement but they are not consecutive to distinguish them from row numbers)
Ideally what you want and what is used all throughout the loris codebase is an array of the form $centerID=>$centerName
as follows
$DB->pselectColWithIndexKey("SELECT CenterID, Name FROM psc WHERE CenterID<11",array(),"CenterID")
Array
(
[1] => CCNA Testing Site
[5] => True North Clinical Research (Halifax)
[6] => St. Joseph's Hospital
[7] => Centre for Diagnosis and Research on Alzheimer's Disease (CEDRA), Memory Clinic
[8] => Centre Hospitalier de l'Université de Montréal
[10] => Douglas Mental Health University Institute, Memory Clinic
)
But currently we can only get one of the following... in either case we will need a secondary forloop to extract the key=>value association we are looking for.
$DB->pselect("SELECT CenterID, Name FROM psc WHERE CenterID<11",array())
Array
(
[0] => Array
(
[CenterID] => 1
[Name] => CCNA Testing Site
)
[1] => Array
(
[CenterID] => 5
[Name] => True North Clinical Research (Halifax)
)
[2] => Array
(
[CenterID] => 6
[Name] => St. Joseph's Hospital
)
[3] => Array
(
[CenterID] => 7
[Name] => Centre for Diagnosis and Research on Alzheimer's Disease (CEDRA), Memory Clinic
)
[4] => Array
(
[CenterID] => 8
[Name] => Centre Hospitalier de l'Université de Montréal
)
[5] => Array
(
[CenterID] => 10
[Name] => Douglas Mental Health University Institute, Memory Clinic
)
)
$DB->pselectCol("SELECT Name FROM psc WHERE CenterID<11",array())
Array
(
[0] => CCNA Testing Site
[1] => True North Clinical Research (Halifax)
[2] => St. Joseph's Hospital
[3] => Centre for Diagnosis and Research on Alzheimer's Disease (CEDRA), Memory Clinic
[4] => Centre Hospitalier de l'Université de Montréal
[5] => Douglas Mental Health University Institute, Memory Clinic
)
$DB->pselectWithIndexKey("SELECT CenterID, Name FROM psc WHERE CenterID<11",array(),"CenterID")
Array
(
[1] => Array
(
[Name] => CCNA Testing Site
)
[5] => Array
(
[Name] => True North Clinical Research (Halifax)
)
[6] => Array
(
[Name] => St. Joseph's Hospital
)
[7] => Array
(
[Name] => Centre for Diagnosis and Research on Alzheimer's Disease (CEDRA), Memory Clinic
)
[8] => Array
(
[Name] => Centre Hospitalier de l'Université de Montréal
)
[10] => Array
(
[Name] => Douglas Mental Health University Institute, Memory Clinic
)
)
I think this function might just be a better version of pselectCol
but I dont think it is redundant with pselectWithIndexKey
because it returns a flatened array directly useable by the code requesting it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@driusan did the explanation make it clearer ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the fact that I didn't see the comment until now, yes.
@driusan Yeah, sorry, I had made comments a week ago and forgot to submit them :/ |
This adds 2 new functions to the database.class.inc that build on the pselect() and pselectCol() in order to return an array indexed by a selected primary key. These functions are meant to replace the multitude of places in the code where the pselect() and pselectCol() result is ran through a foreach loop in order to extract and re-index the array using its primary keys.
The necessary Exceptions are thrown to avoid data being overwritten in the built array and misuse of the function in general