[MDEV-24392] execute immediate '/*M!100601 select 10_6_only_sql */' returns error incorrectly Created: 2020-12-10 Updated: 2023-03-03 |
|
| Status: | Open |
| Project: | MariaDB Server |
| Component/s: | Prepared Statements |
| Fix Version/s: | None |
| Type: | Task | Priority: | Trivial |
| Reporter: | Daniel Black | Assignee: | Daniel Black |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | beginner-friendly, not-10.6+ | ||
| Issue Links: |
|
||||||||||||||||
| Description |
|
execute immediate '/* nothing */' -> "This command is not supported in the prepared statement protocol yet" (tested 10.2.38) Workarounds of doing an empty string work. |
| Comments |
| Comment by Debjyoti Ghosh [ 2023-02-22 ] | ||||||||||||||||||||||||||||||||
|
@Daniel Black, I see that this issue still exists in 11.0.1, I want to contribute in solving this, can you please help me get started with it, like where should i look in the codebase, possible ways to fix this , etc anything would help, (i am a beginner in open source), thanks. | ||||||||||||||||||||||||||||||||
| Comment by Daniel Black [ 2023-02-23 ] | ||||||||||||||||||||||||||||||||
|
It doesn't exist as a problem in 11.0.1:
Setting a breakpoint on my_message shows the stack where the error is emitted is:
| ||||||||||||||||||||||||||||||||
| Comment by Debjyoti Ghosh [ 2023-02-23 ] | ||||||||||||||||||||||||||||||||
|
Thanks for the clarification, i initially executed just this execute immediate ; , and thought if we don't pass anything we get an error.
| ||||||||||||||||||||||||||||||||
| Comment by Daniel Black [ 2023-02-23 ] | ||||||||||||||||||||||||||||||||
|
You can gdb attach to a process by using its arguments -p processid. b my_message will break at that point. r to run. Then execute the query. Interesting on the space containing string error. I'm wondering if [https://mariadb.com/kb/en/prepare-statement/|prepared statement] exhibit the same behaviour. | ||||||||||||||||||||||||||||||||
| Comment by Debjyoti Ghosh [ 2023-02-27 ] | ||||||||||||||||||||||||||||||||
|
Hi, for making code changes to this jira should i build server from 10.2 or 10.4 version branch (as you had earlier mentioned on zulip) | ||||||||||||||||||||||||||||||||
| Comment by Daniel Black [ 2023-02-27 ] | ||||||||||||||||||||||||||||||||
|
10.4 | ||||||||||||||||||||||||||||||||
| Comment by Debjyoti Ghosh [ 2023-02-28 ] | ||||||||||||||||||||||||||||||||
|
Hi @Daniel, I have created a PR for this issue, can you please review it. | ||||||||||||||||||||||||||||||||
| Comment by Alexander Barkov [ 2023-03-01 ] | ||||||||||||||||||||||||||||||||
|
Why do we need empty statement support in dynamic SQL? Does the SQL standard support empty expressions in PREPARE / EXECUTE IMMEDIATE? | ||||||||||||||||||||||||||||||||
| Comment by Daniel Black [ 2023-03-01 ] | ||||||||||||||||||||||||||||||||
|
The case I was looking at was executable comments where the version may not have been the same MariaDB version running. Nothing was expected to be returned if this wasn't the code comment version applicable to the server. Its useful for trying to write portable fixes in open source code targetting multiple versions of MariaDB and non-MariaDB codebases. I think the problem I was looking at was https://github.com/vitessio/vitess/pull/7318, though it might have been something else like MySQLTuner. The standard on https://crate.io/docs/sql-99/en/latest//chapters/44.html#preparable-sql-statements in the Algorithm says <<simple comments> should return a syntax error, however on empty statements its not specified. Note that MariaDB-10.6+ parses an empty EXECUTE IMMEDIATE already, so this is a minor backport of that functionality. | ||||||||||||||||||||||||||||||||
| Comment by Alexander Barkov [ 2023-03-02 ] | ||||||||||||||||||||||||||||||||
|
Portable scripts make sense. However, I think we should try to be consistent in direct execution and prepared execution as much as possible. We need some simple explanation (to put it to the manual after the change) how we handle all these situations:
in all these execution types:
| ||||||||||||||||||||||||||||||||
| Comment by Alexander Barkov [ 2023-03-02 ] | ||||||||||||||||||||||||||||||||
|
Just checked in 10.10: For now, direct execution returns errors:
EXECUTE IMMEDIATE works without errors:
PREPARE..EXECUTE works without errors:
| ||||||||||||||||||||||||||||||||
| Comment by Alexander Barkov [ 2023-03-02 ] | ||||||||||||||||||||||||||||||||
|
danblack, you know which commit allowed empty statements in prepared execution in the latest versions? | ||||||||||||||||||||||||||||||||
| Comment by Daniel Black [ 2023-03-02 ] | ||||||||||||||||||||||||||||||||
|
Found it: | ||||||||||||||||||||||||||||||||
| Comment by Daniel Black [ 2023-03-03 ] | ||||||||||||||||||||||||||||||||
|
Based on bar's evaluation, and the SQL standard, per MDEV-30772, we need to be consistent between prepared and non-prepared statements. so:
So the test case is which should succeed without a return value:
The current 10.3/4/5 behaviours are correct:
|