MDEV-18827 Create utility to parse frm files and print their DDL

[hp77-creator]

Add FRM parser as a standalone utility to mariaDB server

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

Description

The following tests are going to be added and run with mtr:

• simple columns, at least int, text, varchar(value)
• keys, using btree
• keys using rtree
• sinple vcol expressions: a int, x int as (a *2)
• same for DEFAULT: a int default (whatever)
• DEFAULT CURRENT_TIMESTAMP is a special case
• field-level comments, table-level comments

==================

• innodb engine
• more exotic engines
• engine options
• partitions -- currently failing so added with error clause

==================

• Expressions from plugins, like json, Item_geometry_func, inet6 -- currently failing so added with error clause
• Sequences: default(nextval(s))

==================

• Foreign keys in innodb -- foreign key constraints are not printed properly

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?

Consult the documentation on "Writing good test cases".

run

> ./mtr --suite=client --do-test=mariadb-frm

If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.

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.

[FooBarrior]

Now it looks directed to the right way.

I noticed there are some junk files included into PR -- pls rollback them.
Add a test: create a table with a real server and invoke your frm parser with MTR's --exec

[FooBarrior]

Overall looks good, I just commented a few creeps about the style, and I think many declarations might be redundand. (I guess that could be left from our past trials with bringing up the minimal linkage...)

You can test it all as follows:

create table t (x int); # will be executed by server
 --exec $MARIADB_FRM $MYSQL_TEST_DIR/std_data/frm/table_simple.frm
drop table t;

# etc, etc, etc...

to test different storage engine, add in the beginning:

--source include/have_innodb.inc
--source include/have_partition.inc
# and for example
--source include/have_blackhole.inc

What needs to be tested:

• simple columns, at least int, text, varchar(value)
• keys, using btree, using rtree
• sinple vcol expressions: a int, x int as (a *2)
• same for DEFAULT: a int default (whatever)
• DEFAULT CURRENT_TIMESTAMP is a special case
• field-level comments, table-level comments

Later on:

• innodb engine
• more exotic engines
• engine options
• Partitions!!

Later on:

• Expressions from plugins, like json, Item_geometry_func, inet6
• sequences: default(nextval(s))

Later on:

• Foreign keys in innodb

This is the full roadmap remaining, as I see it. I think we won't reach full support for everythink written in time, but at least:

• We shouldn't crash on those cases.
• We need to add an information on what's not supported in the commit message
• A test showing it's not crasing is still needed.

[FooBarrior]

Thank you for your work @hp77-creator!

I really like a steady collection of tests.

About the code - I see some functions handle unexistent error codes, etc, making me think that it's AI generated. If that's so, it should be reverted.

AI is nothing bad in general, but the author should know what are they doing and validate the output.

For supporting the arbitrary expressions, I suggest you examine the following approach

1. Inherit an own Item, like Frm_parser_item
2. This item will do nothing but output an expression string it was supplied with through a constructor, in Frm_parser_item::print()
3. In unpack_vcol_info_from_frm:
   #ifdef FRM_PARSER
   return new Virtual_column_info(new Frm_parser_item(expr_str->c_ptr_safe(), expr_str->length())));
   #endif
   3.1. To make it work, youll have to add table.cc explicitly in the sources list! So that the macro would take effect.
   3.2 Alternatively, make a global variable denoting an frm parser mode
   3.3 Alternatively again, open_table_from_share->...->unpack_vcol_info_from_frm call chaing with an extra argument, denoting frm parser mode. This is the cleanest solution out of proposed.

Should work!

About the storage engines -- given that we already have a working mockup, an arbitrary engine should just work. The question is only in how to test it.

For expample, EXAMPLE storage engine can be enabled with supplying -DWITH_EXAMPLE_STORAGE_ENGINE=1 in the cmake line.

Then have_example_plugin.inc should work in tests after you'll build the target.

[hp77-creator]

About the code - I see some functions handle unexistent error codes, etc, making me think that it's AI generated. If that's so, it should be reverted.

AI is nothing bad in general, but the author should know what are they doing and validate the output.

100% agree, I didn't validate everything it gave and just pushed it, sorry for that, I will go through the comments and fix whatever's hallucinated.

[hp77-creator]

@FooBarrior done.

[hp77-creator]

Hey @FooBarrior any plans for this PR?

[FooBarrior]

@hp77-creator yes it's queued for adoption and merge

[gkodinov]

This seems to have fallen through and didn't get merged. I'm terribly sorry for that on behalf of the project.
And I fully intend to get it moving again.
I'd like to ask you one more thing before we proceed with the testing required: Can you please rebase on main? This is a new feature and it needs to go to that branch.

[hp77-creator]

Hi @gkodinov , shall I rebase it from MariaDB/server main and then force push the rebased commit?
Although there's only one commit, I am getting conflicts when I am trying to push the local branch which is rebased with MariaDB/server main

[hp77-creator]

i also changed the PR's base branch to main since staging one was 920 commits behind the current main, let me know if something else needs to be done

[gkodinov]

You did the rebase correctly. Unfortunately, when doing that UI alone won't do it (afaik). You also need to rebase your branch by yourself.

Please ensure that the buildbot tests are passing. Right now there's a compilation error:

/home/buildbot/aarch64-debian-11/build/sql/table.cc: In function ‘bool parse_vcol_defs(THD*, MEM_ROOT*, TABLE*, bool*, vcol_init_mode, bool)’:
/home/buildbot/aarch64-debian-11/build/sql/table.cc:1273:96: error: too many arguments to function ‘Virtual_column_info* unpack_vcol_info_from_frm(THD*, TABLE*, String*, Virtual_column_info**, bool*)’
1273 |                                     &((*field_ptr)->vcol_info), error_reported, frm_parser_mode);

[gkodinov]

Do you intend to keep working on this? It's been 3 weeks since I've supplied review comments. Changing to draft due to inactivity. Please change it back when/if you are ready with the next batch.

[hp77-creator]

Do you intend to keep working on this? It's been 3 weeks since I've supplied review comments. Changing to draft due to inactivity. Please change it back when/if you are ready with the next batch.

yes, I inted to get this merged. sorry for delay. I was occupied in some work stuff. will be more active here

[FooBarrior]

@hp77-creator there is a (unintended) concatenation of various commit messages in your commit message, please check

[hp77-creator]

@hp77-creator there is a (unintended) concatenation of various commit messages in your commit message, please check

snap! I did multiple git rebase, probably that appended messages of every instant, i will amend it, thanks @FooBarrior

[hp77-creator]

some tests are failing, converting to draft to resolve those. will make it RFR once done.