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.

Sign up for GitHub

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

Jump to bottom

Rephrased all SQL queries to follow Moodle SQL style guidelines #89

Closed
wants to merge 2 commits into from
Closed

Rephrased all SQL queries to follow Moodle SQL style guidelines #89

wants to merge 2 commits into from

Conversation

dionysius
Copy link
Member

@dionysius dionysius commented Feb 4, 2019 •
edited
Loading

From MDL-62822

task/delete_quiz_after_migration.php
SQL query does not follow moodle SQL guidelines.

  • this commit hasn't changed any query
    • Only worth mentioning: Simplified the function classes\question\bank\tag_column.php get_extra_joins(). The queries were too identical
  • updated all queries (which are not in tests) to follow the Moodle SQL style guidelines
  • subqueries guidelines are not described on the offial developers page. Followed this comment as it recieved the most acknowledgement
  • some findings:
    • classes\privacy\provider.php is full of $DB->execute, maybe change them, where applicable, to corresponding insert/delete/update functions
    • some places have LIKE, CONCAT statements and such, we should use the compatibility functions as much as possible
    • some places have have inline variable use in SQL queries (search for " \..*\. " with regex over the files)

@dionysius
Rephrased all SQL queries to follow Moodle SQL style guidelines
75f8c24 
From [MDL-62822](https://tracker.moodle.org/browse/MDL-62822?focusedCommentId=647722&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-647722)

> task/delete_quiz_after_migration.php
> [SQL query](lameze/moodle@cb7f6a6...MDL-62822-master#diff-eb39085ab85a788c6d8d534538a64140R45) does not follow moodle SQL guidelines.

- this commit hasn't changed any query
  - Only worth mentioning: Simplified the function classes\question\bank\tag_column.php get_extra_joins(). The queries were too identical
- updated all queries (which are not in tests) to follow the [Moodle SQL style guidelines](https://docs.moodle.org/dev/SQL_coding_style)
- subqueries guidelines are not described on the offial developers page. Followed [this comment](https://tracker.moodle.org/browse/MDLSITE-1914?focusedCommentId=177169&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-177169) as it recieved the most acknowledge
- some findings:
  - classes\privacy\provider.php is full of $DB->execute, maybe change them, where applicable, to corresponding insert/delete/update functions
  - some places have LIKE, CONCAT statements and such, we should use [the compatibility functions](https://docs.moodle.org/dev/Data_manipulation_API#SQL_compatibility_functions) as much as possible
  - some places have have inline variable use in SQL queries (search for `" \..*\. "` with regex over the files)
@dionysius
minor fixes to sql coding style
2eb67a6
@muqiuq muqiuq closed this Feb 8, 2019
@dionysius dionysius deleted the MDL-62822-sqlbyguidelines branch March 10, 2019 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dionysius @muqiuq