MDEV-38194 Correct classification of MASTER_SSL_VERIFY_SERVER_CERT

[deo002]

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

Description

The issue is that IS.slave_status column Master_SSL_Verify_Server_Cert requires identifier quoting. No other CHANGE MASTER field requires quoting.

This patch removes MASTER_SSL_VERIFY_SERVER_CERT from the reserved keyword list reserved_keyword_udt_not_param_type and adds it to keyword_func_sp_var_and_label in the parser. All other MASTER_SSL_* keywords are in this list.

This allows MASTER_SSL_VERIFY_SERVER_CERT to be used unquoted.

How can this PR be tested?

Before the fix, this failed:

SELECT Master_SSL_Verify_Server_Cert FROM information_schema.slave_status;

After the fix, this must succeed without quoting:

SELECT Master_SSL_Verify_Server_Cert FROM information_schema.slave_status;

And still work with quoting:

SELECT `Master_SSL_Verify_Server_Cert` FROM information_schema.slave_status;

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.

[CLAassistant]

All committers have signed the CLA.

[bnestere]

Hi @deo002 !

Thank you for your contribution! I have three requests:

First, can you please add in an MTR test (documentation link) to show the fix?

I'd suggest creating a new test in the rpl suite, say rpl_is_slave_status.test (the is to abbreviate information_schema. An existing MTR test you could use to get started would be rpl_init_slave.test

Second, to your git commit message, I'd extend

This allows MASTER_SSL_VERIFY_SERVER_CERT to be used unquoted.

to:

This allows MASTER_SSL_VERIFY_SERVER_CERT to be used unquoted when directly accessed via INFORMATION_SCHEMA.slave_status, as well as opens it up to be used as names in other places, e.g. stored procedures.

And lastly, you have a failing test that you can see in the checks section of the PR. You can click each builder which failed, and investigate the failed test by looking at the failure output. I.e, look at the failing step (in this case it is test), download the stdio output file, and search the file for [ fail ] to find the failing test and why it failed.

[ParadoxV5]

First, can you please add in an MTR test (documentation link) to show the fix?

I'd suggest creating a new test in the rpl suite, say rpl_is_slave_status.test (the is to abbreviate information_schema. An existing MTR test you could use to get started would be rpl_init_slave.test

The MDEV-33856 supertask must have had no MTR or QA testing specific to the MDEV-33526 “Create IS.slave_status table” subtask; this issue would’ve easily been found otherwise.

Second, to your git commit message, I'd extend

This allows MASTER_SSL_VERIFY_SERVER_CERT to be used unquoted.

to:

This allows MASTER_SSL_VERIFY_SERVER_CERT to be used unquoted when directly accessed via INFORMATION_SCHEMA.slave_status, as well as opens it up to be used as names in other places, e.g. stored procedures.

These two halves can further merge.

This allows using master_ssl_verify_server_cert as an unquoted identifier, namely when SELECTing from INFORMATION_SCHEMA.slave_status.

[deo002]

Thanks for the help @bnestere ! I have completed the requested changes.

[bnestere]

Hi @deo002 !

This added test is great! Thanks for testing each use of master_ssl_verify_server_cert to ensure it can be used in other places :)

Overall the patch is approved, with a couple minor changes. For the "remove space from end of line" comments, before submitting a patch, I'd recommend reviewing the patch content via git show on the command-line and it will show things like extra whitespace, etc.

Thank you for the contribution!

Also sorry for the long delay in this review, I didn't see that you'd pushed updates. Happy holidays!

[deo002]

Done! Thanks! Happy Holidays!

[bnestere]

Looks good, @deo002 !

One last minor request would be to add the line

Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>

to the end of your git commit message.

Thank you for your contribution!

[deo002]

Done!