[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:
Blocks
is blocked by MDEV-30772 Inconsistency in handling empty comme... Open
Relates
relates to MDEV-16708 Unsupported commands for prepared sta... Closed

 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:

Server version: 11.0.2-MariaDB Source distribution
 
Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.
 
Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.
 
MariaDB [(none)]> execute immediate '/* nothing */';
Query OK, 0 rows affected (0.000 sec)

Setting a breakpoint on my_message shows the stack where the error is emitted is:

10.3

(gdb) bt
#0  my_message (error=1295, str=0xe0ee2f "This command is not supported in the prepared statement protocol yet", MyFlags=0) at /home/dan/repos/mariadb-server-10.3/mysys/my_error.c:196
#1  0x000000000064a6f3 in check_prepared_statement (stmt=0x7f93340166f8) at /home/dan/repos/mariadb-server-10.3/sql/sql_prepare.cc:2517
#2  Prepared_statement::prepare (this=this@entry=0x7f93340166f8, packet=<optimized out>, packet_len=<optimized out>) at /home/dan/repos/mariadb-server-10.3/sql/sql_prepare.cc:4240
#3  0x000000000064b2e9 in Prepared_statement::execute_immediate (this=0x50f, this@entry=0x7f93340166f8, query=0xe0ee2f "This command is not supported in the prepared statement protocol yet", query_len=0) at /home/dan/repos/mariadb-server-10.3/sql/sql_prepare.cc:5143
#4  0x000000000064b196 in mysql_sql_stmt_execute_immediate (thd=thd@entry=0x7f9334000c58) at /home/dan/repos/mariadb-server-10.3/sql/sql_prepare.cc:2989
#5  0x0000000000638ff5 in mysql_execute_command (thd=thd@entry=0x7f9334000c58) at /home/dan/repos/mariadb-server-10.3/sql/sql_parse.cc:3877
#6  0x0000000000634528 in mysql_parse (thd=thd@entry=0x7f9334000c58, rawbuf=0x7f933400f680 "execute immediate '/* nothing */'", length=<optimized out>, parser_state=parser_state@entry=0x7f93880b26e0, is_com_multi=false, is_next_command=false) at /home/dan/repos/mariadb-server-10.3/sql/sql_parse.cc:7855
#7  0x0000000000631665 in dispatch_command (command=command@entry=COM_QUERY, thd=thd@entry=0x7f9334000c58, packet=packet@entry=0x7f93340073c9 "execute immediate '/* nothing */'", packet_length=packet_length@entry=33, is_com_multi=false, is_next_command=false) at /home/dan/repos/mariadb-server-10.3/sql/sql_parse.cc:1852
#8  0x00000000006333bf in do_command (thd=0x7f9334000c58) at /home/dan/repos/mariadb-server-10.3/sql/sql_parse.cc:1398
#9  0x0000000000708df9 in do_handle_one_connection (connect=<optimized out>) at /home/dan/repos/mariadb-server-10.3/sql/sql_connect.cc:1404
#10 0x0000000000708c23 in handle_one_connection (arg=0x24224d8) at /home/dan/repos/mariadb-server-10.3/sql/sql_connect.cc:1309
#11 0x00007f939daae12d in start_thread () from /lib64/libc.so.6
#12 0x00007f939db2fbc0 in clone3 () from /lib64/libc.so.6
 
(gdb) up
#1  0x000000000064a6f3 in check_prepared_statement (stmt=0x7f93340166f8) at /home/dan/repos/mariadb-server-10.3/sql/sql_prepare.cc:2517
2517	    if (lex->create_view->mode == VIEW_ALTER)
(gdb)  p lex->create_view->mode
Cannot access memory at address 0x10
(gdb)  p lex->create_view
$2 = (Create_view_info *) 0x0

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.
Also, can you guide me on how did you get the stack trace using gdb "Setting a breakpoint on my_message shows the stack where the error is emitted is:".

MariaDB [(none)]> execute immediate '/* nothing */';
Query OK, 0 rows affected (0.000 sec)
 
MariaDB [(none)]> execute immediate ' ';
ERROR 1065 (42000): Query was empty
MariaDB [(none)]> execute immediate ;
ERROR 1243 (HY000): Unknown prepared statement handler (immediate) given to EXECUTE
MariaDB [(none)]>
MariaDB [(none)]>

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.
PR link

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:

  • Completely empty statements
  • Empty statements consisting only of non-executable comments
  • Statements consisting only of executible comments which are effectively empty because of the version

in all these execution types:

  • Direct execution
  • Prepared execution (PREPARE/EXECUTE and EXECUTE IMMEDIATE)
Comment by Alexander Barkov [ 2023-03-02 ]

Just checked in 10.10:

For now, direct execution returns errors:

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

EXECUTE IMMEDIATE works without errors:

MariaDB [test]> execute immediate '/* */';
Query OK, 0 rows affected (0.001 sec)

PREPARE..EXECUTE works without errors:

MariaDB [test]> PREPARE stmt FROM '/* */';
Query OK, 0 rows affected (0.000 sec)
Statement prepared
 
MariaDB [test]> EXECUTE stmt;
Query OK, 0 rows affected (0.000 sec)

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: MDEV-16708 9370c6e83c148b4a5d4f08de7778e6a02da6adcb where the default condition changed to just a break.

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:

  • comments aren't allowed
  • executable comments that evaluate to empty should be allowed.

So the test case is which should succeed without a return value:

MariaDB [test]> execute immediate '/*M!999999 should be no error */';
ERROR 1295 (HY000): This command is not supported in the prepared statement protocol yet

The current 10.3/4/5 behaviours are correct:

MariaDB [test]> execute immediate '';
ERROR 1065 (42000): Query was empty
MariaDB [test]> execute immediate '/* nothing */';
ERROR 1295 (HY000): This command is not supported in the prepared statement protocol yet

Generated at Thu Feb 08 09:29:38 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.