MDEV-24392 Added support for EXECUTE_IMMEDIATE command when empty str…

[is-this-echo]

trafficstars

• [x] The Jira issue number for this PR is: MDEV-24392

Description

TODO: Handled the case when passing a comment inside the EXECUTE_IMMEDIATE command throws an error : ERROR 1243 (HY000): Unknown prepared statement handler (immediate) given to EXECUTE

Modified the default behaviour of an unknown prepared_statement to throw error message when comments are passed inside the EXECUTE_STATEMENT command. Another workaround could be to handle the passed sql_command in the switch case but couldn't find anything suitable for comments in sql/sql_cmd.h file.

How can this PR be tested?

Test and Result files are added to the mysql-test dir. This PR can be tested by running : ./mtr execute_immediate_empty

Basing the PR against the correct MariaDB version

• [ ] This is a new feature and the PR is based against the latest MariaDB development branch
• [x] This is a bug fix and the PR is based against the earliest maintained branch in which the bug can be reproduced

Backward compatibility

TODO: ------

PR quality check

• [x] I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.

[CLAassistant]

All committers have signed the CLA.

[Weijun-H]

Hi @is-this-echo, could you please take a look at the test that's failing?

[grooverdan]

Setting the breakpoint in the check_prepared_statement for the test case shows the sql_command to be SQLCOM_EMPTY_QUERY. Please change break on this only and not change the default. This should fix the other test results as evidently the enumeration of SQLCOM here isn't complete.

Rather than a new test file, can you append the test to before the "end of 10.4 tests" in mysql-test/main/ps.test.

After those changes:

• git rm -f the previously added test file and result
• git add the ps.test and ps.result file, and sql/sql_prepare.cc
• git commit --amend (to include added files into current commit). Recheck wording in commit message.
• git show to see if the change is minimal and correct
• git push -f to update the remote branch (which auto updates this PR).

[is-this-echo]

Thanks for pointing towards the correct solution, I was not aware of the SQLCOM_EMPTY_QUERY sql_command, attached gdb debugger to server and put a breakpoint at my_message(), realized it was hitting the default case in switch statement in check_prepared_statement() as the query was unknown.

Could you please share the command/process to attach gdb to the mtr test and set appropriate breakpoint...

[grooverdan]

From your breakpoint, you can go up and print variable that exist in the stack point.

In using mtr however there is the following where the optional args to gdb are the commands to execute.:

mysql-test/mtr --gdb='b check_prepared_statement;r' main.execute_immediate

Needs xterm installed.

[is-this-echo]

understood, thanks. Please let me know if some further changes are required in this PR..

[grooverdan]

Appoligies for the late concept review. As @abarkov rightly points out in MDEV-30772, we need to be consistent between output in prepared statements vs non-prepared statements.

10.6, as a result of the unintential consequences of implementing MDEV-16708, started incorrectly accepting sole comments as allowed prepared statements in contradiction to the non-prepared statements and the SQL-99 standard.

If you'd like adjust this PR to be "Add support for empty evaluated executable syntax" per added comment on the MDEV-24392 that would be appreciated.

If you want to just park this for now and attempt the MDEV-30772 fix in 10.6+, where the default: in check_prepared_statement is break that's also most welcome. Please do comment on MDEV-30772 if you're attempting to advice @dmitryshulga before a second implementation starts.

Again, apologies for leading you towards an problem that wasn't correctly defined.

[is-this-echo]

ok, according to MDEV-30772, for 10.6+ versions, Prepared-execution should behave as given below :

MariaDB [test]> EXECUTE IMMEDIATE '/*M!200101 MariaDB-10.x ignores this */';
Query OK, 0 rows affected (0.001 sec)

MariaDB [test]> EXECUTE IMMEDIATE '/* */';
ERROR: No query specified

In that case, changing the SQLCOM_EMPTY_QUERY to break wouldn't work as they would produce the same result for above cases. For Direct-execution, empty query is handled by client/mysql.cc : com_go() , need to check the flow.

Also, for this PR, as per my understanding, this should be the scenario : Executable comments that evaluate to empty should be allowed(according to special comment format) , other comments should return : ERROR 1295 (HY000): This command is not supported in the prepared statement protocol yet

So, this is expected here :

MariaDB [test]> execute immediate '/*M!999999 should be no error */';
Query OK, 0 rows affected (0.001 sec)

Please correct me if i am wrong.

[grooverdan]

That is correct.

[iangilfillan]

@is-this-echo Are you still interested in wrapping this up?

[tonychen2001]

A couple things to clean up when the author decides to update this.

• This should be re-based onto 10.5 due to 10.4 being near EoL and the final release is typically reserved for critical / secutiry fixes.
• Update the PR description
  • Update and remove areas marked with TODO:
  • The PR description and the changes in the commit do not match up
  • The How can this PR be tested? section is outdated and should be updated to reflect that a test case was added to the main.ps MTR