Skip to content

MDEV-27013: COM_STMT_PREPARE returns 0 columns for INSERT .. RETURNING #4045

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: 10.6
Choose a base branch
from
Open

MDEV-27013: COM_STMT_PREPARE returns 0 columns for INSERT .. RETURNING #4045

wants to merge 1 commit into from

Conversation

grooverdan
Copy link
Member

@grooverdan grooverdan commented May 15, 2025 •
edited
Loading

  • The Jira issue number for this PR is: MDEV-27013

Description

If this was a SELECT statement, the columns would be populated by the mysql_test_insert call to send_prep_stmt. So we need to get the column count for returning statements and populate this instead of 0.

Release Notes

TODO: What should the release notes say about this change?
Include any changed system variables, status variables or behaviour. Optionally list any https://mariadb.com/kb/ pages that need changing.

How can this PR be tested?

$ socat -x -v UNIX-LISTEN:/tmp/intercept.sock,fork UNIX-CONNECT:/tmp/build-mariadb-server-rebase.sock  

$ tests/mariadb-client-test -S /tmp/intercept.sock  -u dan -D test  test_mdev_27013


#####################################
client_connect  
#####################################

 Establishing a connection to '' ...OK
Connected to MySQL server version: 10.6.22-MariaDB (100622)

 Creating a test database 'client_test_db' ...OK


#####################################
1 of (1/1): test_mdev27013  
#####################################
err: Unknown or undefined error code/home/dan/repos/mariadb-server-rebase/tests/mysql_client_test.c:21902: check failed: 'rc == 0'

> 2025/05/15 17:15:46.000580099  length=82 from=485 to=566
 05 00 00 00 19 01 00 00 00 37 00 00 00 16 49 4e  .........7....IN
 53 45 52 54 20 49 4e 54 4f 20 6d 64 65 76 32 37  SERT INTO mdev27
 30 31 33 20 28 63 31 29 20 56 41 4c 55 45 53 28  013 (c1) VALUES(
 27 74 77 65 65 74 27 29 20 52 45 54 55 52 4e 49  'tweet') RETURNI
 4e 47 20 2a 0a                                   NG *.
 00 00 00 17 ff ff ff ff 00 01 00 00 00           .............
--
< 2025/05/15 17:15:46.000580299  length=16 from=430 to=445
 0c 00 00 01 00 02 00 00 00 02 00 00 00 00 00 00  ................
--
< 2025/05/15 17:15:46.000580330  length=162 from=446 to=607
 02 00 00 01 02 01 3b 00 00 02 03 64 65 66 0e 63  ......;....def.c
 6c 69 65 6e 74 5f 74 65 73 74 5f 64 62 09 6d 64  lient_test_db.md
 65 76 32 37 30 31 33 09 6d 64 65 76 32 37 30 31  ev27013.mdev2701
 33 02 63 31 02 63 31 00 0c 2d 00 90 00 00 00 fe  3.c1.c1..-......
 00 00 00 00 00 3b 00 00 03 03 64 65 66 0e 63 6c  .....;....def.cl
 69 65 6e 74 5f 74 65 73 74 5f 64 62 09 6d 64 65  ient_test_db.mde
 76 32 37 30 31 33 09 6d 64 65 76 32 37 30 31 33  v27013.mdev27013
 02 63 32 02 63 32 00 0c 2d 00 90 00 00 00 fe 00  .c2.c2..-.......
 00 00 00 00 05 00 00 04 fe 00 00 02 00 08 00 00  ................
 05 00 08 05 74 77 65 65 74 05 00 00 06 fe 00 00  ....tweet.......
 02 00                                            ..
--

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@grooverdan grooverdan requested a review from sanja-byelkin May 15, 2025 07:19
@grooverdan grooverdan requested a review from 9EOR9 May 15, 2025 07:25
@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label May 15, 2025
@rusher
Copy link
Member

rusher commented May 15, 2025 •
edited
Loading

The problem with current correction is that the expected number of columns is returned, but no columns follows.
see https://mariadb.com/kb/en/com_stmt_prepare/ : for you example, after "0c 00 00 01 00 02 00 00 00 02 00 00 00 00 00 00" that corresponds to COM_STMT_PREPARE_OK, 2 column definition packet are expected.

see a few line above : result.send_result_set_metadata(field_list, Protocol::SEND_EOF);

@grooverdan
Copy link
Member Author

grooverdan commented May 16, 2025 •
edited
Loading

Thanks @rusher

The comm list matches the list in sp_get_flags_for_command.

Ready:

> 2025/05/16 14:33:18.000324309  length=55 from=485 to=539
 05 00 00 00 19 01 00 00 00 1c 00 00 00 16 53 45  ..............SE
 4c 45 43 54 20 63 31 2c 63 32 20 46 52 4f 4d 20  LECT c1,c2 FROM 
 6d 64 65 76 32 37 30 31 33 0a                    mdev27013.
 00 00 00 17 ff ff ff ff 00 01 00 00 00           .............
--
< 2025/05/16 14:33:18.000324452  length=151 from=430 to=580
 0c 00 00 01 00 02 00 00 00 02 00 00 00 00 00 00  ................
 3b 00 00 02 03 64 65 66 0e 63 6c 69 65 6e 74 5f  ;....def.client_
 74 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31  test_db.mdev2701
 33 09 6d 64 65 76 32 37 30 31 33 02 63 31 02 63  3.mdev27013.c1.c
 31 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 3b  1..-...........;
 00 00 03 03 64 65 66 0e 63 6c 69 65 6e 74 5f 74  ....def.client_t
 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31 33  est_db.mdev27013
 09 6d 64 65 76 32 37 30 31 33 02 63 32 02 63 32  .mdev27013.c2.c2
 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 05 00  ..-.............
 00 04 fe 00 00 02 00                             .......
--
< 2025/05/16 14:33:18.000324703  length=24 from=581 to=604
 02 00 00 01 02 00 05 00 00 02 fe 00 00 22 00 05  ............."..
 00 00 03 fe 00 00 22 00                          ......".
--
> 2025/05/16 14:33:18.000324760  length=82 from=540 to=621
 05 00 00 00 19 02 00 00 00 37 00 00 00 16 49 4e  .........7....IN
 53 45 52 54 20 49 4e 54 4f 20 6d 64 65 76 32 37  SERT INTO mdev27
 30 31 33 20 28 63 31 29 20 56 41 4c 55 45 53 28  013 (c1) VALUES(
 27 74 77 65 65 74 27 29 20 52 45 54 55 52 4e 49  'tweet') RETURNI
 4e 47 20 2a 0a                                   NG *.
 00 00 00 17 ff ff ff ff 00 01 00 00 00           .............
--
< 2025/05/16 14:33:18.000324956  length=151 from=605 to=755
 0c 00 00 01 00 03 00 00 00 02 00 00 00 00 00 00  ................
 3b 00 00 02 03 64 65 66 0e 63 6c 69 65 6e 74 5f  ;....def.client_
 74 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31  test_db.mdev2701
 33 09 6d 64 65 76 32 37 30 31 33 02 63 31 02 63  3.mdev27013.c1.c
 31 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 3b  1..-...........;
 00 00 03 03 64 65 66 0e 63 6c 69 65 6e 74 5f 74  ....def.client_t
 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31 33  est_db.mdev27013
 09 6d 64 65 76 32 37 30 31 33 02 63 32 02 63 32  .mdev27013.c2.c2
 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 05 00  ..-.............
 00 04 fe 00 00 02 00                             .......
--
< 2025/05/16 14:33:18.000325220  length=36 from=756 to=791
 02 00 00 01 02 00 05 00 00 02 fe 00 00 02 00 08  ................
 00 00 03 00 08 05 74 77 65 65 74 05 00 00 04 fe  ......tweet.....
 00 00 02 00                                      ....
--
> 2025/05/16 14:33:18.000325300  length=82 from=622 to=703
 05 00 00 00 19 03 00 00 00 37 00 00 00 16 52 45  .........7....RE
 50 4c 41 43 45 20 49 4e 54 4f 20 6d 64 65 76 32  PLACE INTO mdev2
 37 30 31 33 20 53 45 54 20 63 32 3d 27 67 72 6f  7013 SET c2='gro
 77 6c 27 20 52 45 54 55 52 4e 49 4e 47 20 63 31  wl' RETURNING c1
 2c 20 63 32 0a                                   , c2.
 00 00 00 17 ff ff ff ff 00 01 00 00 00           .............
--
< 2025/05/16 14:33:18.000325477  length=151 from=792 to=942
 0c 00 00 01 00 04 00 00 00 02 00 00 00 00 00 00  ................
 3b 00 00 02 03 64 65 66 0e 63 6c 69 65 6e 74 5f  ;....def.client_
 74 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31  test_db.mdev2701
 33 09 6d 64 65 76 32 37 30 31 33 02 63 31 02 63  3.mdev27013.c1.c
 31 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 3b  1..-...........;
 00 00 03 03 64 65 66 0e 63 6c 69 65 6e 74 5f 74  ....def.client_t
 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31 33  est_db.mdev27013
 09 6d 64 65 76 32 37 30 31 33 02 63 32 02 63 32  .mdev27013.c2.c2
 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 05 00  ..-.............
 00 04 fe 00 00 02 00                             .......
--
< 2025/05/16 14:33:18.000325737  length=36 from=943 to=978
 02 00 00 01 02 00 05 00 00 02 fe 00 00 02 00 08  ................
 00 00 03 00 04 05 67 72 6f 77 6c 05 00 00 04 fe  ......growl.....
 00 00 02 00                                      ....
--
> 2025/05/16 14:33:18.000325851  length=61 from=704 to=764
 05 00 00 00 19 04 00 00 00 22 00 00 00 16 44 45  ........."....DE
 4c 45 54 45 20 46 52 4f 4d 20 6d 64 65 76 32 37  LETE FROM mdev27
 30 31 33 20 52 45 54 55 52 4e 49 4e 47 20 2a 0a  013 RETURNING *.
 00 00 00 17 ff ff ff ff 00 01 00 00 00           .............
--
< 2025/05/16 14:33:18.000325958  length=151 from=979 to=1129
 0c 00 00 01 00 05 00 00 00 02 00 00 00 00 00 00  ................
 3b 00 00 02 03 64 65 66 0e 63 6c 69 65 6e 74 5f  ;....def.client_
 74 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31  test_db.mdev2701
 33 09 6d 64 65 76 32 37 30 31 33 02 63 31 02 63  3.mdev27013.c1.c
 31 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 3b  1..-...........;
 00 00 03 03 64 65 66 0e 63 6c 69 65 6e 74 5f 74  ....def.client_t
 65 73 74 5f 64 62 09 6d 64 65 76 32 37 30 31 33  est_db.mdev27013
 09 6d 64 65 76 32 37 30 31 33 02 63 32 02 63 32  .mdev27013.c2.c2
 00 0c 2d 00 90 00 00 00 fe 00 00 00 00 00 05 00  ..-.............
 00 04 fe 00 00 02 00                             .......
--
< 2025/05/16 14:33:18.000326214  length=48 from=1130 to=1177
 02 00 00 01 02 00 05 00 00 02 fe 00 00 22 00 08  ............."..
 00 00 03 00 08 05 74 77 65 65 74 08 00 00 04 00  ......tweet.....
 04 05 67 72 6f 77 6c 05 00 00 05 fe 00 00 22 00  ..growl.......".

@grooverdan grooverdan marked this pull request as ready for review May 16, 2025 04:35
@grooverdan grooverdan removed the request for review from 9EOR9 May 16, 2025 04:35
@grooverdan
MDEV-27013: COM_STMT_PREPARE returns 0 columns for INSERT/REPLACE/DEL…
b2b1a26 
…ETE .. RETURNING

To satisfy the protocol requirements the SQL statements with RETURNING,
the protocol needed to indicate the number of columns returned. Once there
the columns returned is indicated, then the column definition packet is
required to follow in the protocol.

To achieve this the check_prepared_statement for the specific commands
that support RETURNING, and have been parsed to have returning now
include the column count followed by the metadata.

INSERT .. SELECT ... RETURNING and REPLACE .. SELECT .. RETURNING previously
didn't populate the fields required. Where a null sel_res is passed to the
function mysql_insert_select_prepare it was an non-prepared statement with
without RETURNING or it might be a prepared statement with returning.
In that case, prepare the result returning fields. Without this MariaDB would
segfault in result.send_result_set_metadata as the item.field items where null.

Note the unit test would return the correct results the server correction
changed as it was populated from the COM_STMT_EXECUTE rather than the
COM_STMT_PREPARE packet.

This test useful to run via socat interceptor[1] to examine the
results at offset 9 after packet beginning 0c along with the fields
that are returned afterwards. Also it tests the required server code
paths.

tests/mariadb-client-test -S /tmp/intercept.sock  -u $USER -D test  test_mdev_27013

[1] socat -x -v UNIX-LISTEN:/tmp/intercept.sock,fork
UNIX-CONNECT:/tmp/mariadb.sock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanja-byelkin sanja-byelkin Awaiting requested review from sanja-byelkin

Assignees

@sanja-byelkin sanja-byelkin

Labels
MariaDB Foundation Pull requests created by MariaDB Foundation
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@grooverdan @rusher @sanja-byelkin