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

Commented out SQL in cleaner_custom_sql_post may run anyway? (but fails when arguments required) #163

Open
jwcatau opened this issue Oct 17, 2023 · 1 comment

Comments

@jwcatau
Copy link

jwcatau commented Oct 17, 2023

I think there's a bug in the SQL parsing, but I haven't traced it down yet.

SQL statements commented out still prepare and execute, but it takes arguments, then it runs the SQL as if the arguments are commented out/empty, resulting in the following error:

Database transaction aborted automatically in /var/www/site/local/datacleaner/cli/clean.php
!!! Error writing to database !!!
Default exception handler: Error writing to database Debug: ERROR:  bind message supplies 2 parameters, but prepared statement "" requires 0
-- My Commented-Out SQL
-- UPDATE mdl_config_plugins set value = $1 where name = $2
[array (
  0 => 'XXXXXX',
  1 => 'YYYYYY',
)]

Despite it being commented out in the Environment Matrix, it still executes, but the transaction failed in this case.

Without looking at the code yet, this error and the data in the original Environment Matrix for this case seemed to suggest that that SQL that was commented out in the Environment Matrix for the Post custom SQL would still be interpreted and run, but fail if it had arguments - because it would detect the SQL to run, presumably create a stored procedure for it, but then try to fill the parameters from the lines that were commented out - so no variables would be provided and the Post custom SQL would fail.

WORKAROUND:
Remove commented out SQL from the Environment Matrix Post (and Pre?) custom SQL commands, and store them elsewhere.

@brendanheywood
Copy link
Contributor

This isn't a data cleaner bug this is a long standing bug in core because the sql preparser in the dml layer is very simplistic and picks up the token in comments incorrectly

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

No branches or pull requests

2 participants