Fix linter for sql query box with multiple statements #19121
Conversation
| } elseif (! empty($options['eventEditor'])) { | ||
| $sqlQuery = 'CREATE EVENT `a` ON SCHEDULE EVERY MINUTE DO ' . $sqlQuery; | ||
| $sqlQuery = "DELIMITER $$\nCREATE EVENT `a` ON SCHEDULE EVERY MINUTE DO " . $sqlQuery . '$$'; |
There was a problem hiding this comment.
Do we need the $$ at the end?
There was a problem hiding this comment.
Yes, multi line content would break otherwise
There was a problem hiding this comment.
I don't think it is necessary, In these cases only one statement should be in the editor and a delimiter at the very end is not required.
There was a problem hiding this comment.
I'm not sure how multiline content would break. I am testing it right now. A delimiter is supposed to delimit two queries apart. If there is no other query appended to it then the delimiter should is redundant.
There was a problem hiding this comment.
I can't get it to break and it seems to work fine without it. The reported issue is also gone. I think we should merge it without the delimiter at the end. It was needed before because the delimiter was reset back to ; but this is also unnecessary.
There was a problem hiding this comment.
Can you state versions for the record? "needed before"
Always setting the delimiter to $$ is not right in normal sql editor. This breaks usage of the default delimiter ; and using DELIMITER in the sql. Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>
MoonE
force-pushed
the
lint-delimiter
branch
2 times, most recently
from
758a7db
to
3abe6fe
Compare
|
The second commit fixes #18389, the problem was that the annotations were generated for non-existing lines as the linted sql was not the same as the sql in the editor. With this fixed, linting routines/events/triggers never (except when the sql length exceeds 10000 characters) produces lint results as the body of these statements is not analyzed. It is simply skipped: Routine: https://github.com/phpmyadmin/sql-parser/blob/011fa18a4e55591fac6545a821921dd1d61c6984/src/Statements/CreateStatement.php#L719-L726 So I'm not sure what the reason was for adding the sql prefix in the first place. Wouldn't it work better without it? Parsing the body of these statements as normal sql. |
I have asked myself this question this morning too. I suppose there was a scenario which made this fix necessary. It seems to be related to this fix phpmyadmin/sql-parser#438 |
That's correct |
|
When the sql string to lint isa changed, the result positions need to be adjusted accordingly. Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>
Signed-off-by: William Desportes <williamdes@wdes.fr>
Description
Only change the delimiter for routine/event/trigger editor. The query editor should use the default delimiter.
Fixes #18389, fixes #18487, fixes #18511,