server icon indicating copy to clipboard operation
server copied to clipboard

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

Open hp77-creator opened this issue 5 months ago • 10 comments

Add FRM parser as a standalone utility to mariaDB server

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

Description

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

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

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

  • [x] innodb engine
  • [X] more exotic engines
  • [X] 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
  • [x] Sequences: default(nextval(s))

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

  • [x] 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

  • [x] 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

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

hp77-creator avatar Jun 08 '25 11:06 hp77-creator

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 03 '25 11:07 CLAassistant

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 avatar Aug 30 '25 02:08 hp77-creator

Hi @hp77-creator !

MTR will complain about mariadb-frm when invoked after installing the packages as in: https://buildbot.mariadb.org/#/builders/889/builds/1596

mysql-test-run: *** ERROR: Could not find any of /usr/extra/mariadbfrm/mariadb-frm /usr/extra/mariadbfrm/mariadb-frm /usr/bin/mariadbfrm/mariadb-frm

I don't have a lot of server knowledge, but I'm sure @FooBarrior can help you with this.

RazvanLiviuVarzaru avatar Sep 07 '25 15:09 RazvanLiviuVarzaru

Hi @RazvanLiviuVarzaru Thanks for noting that, actually I had fixed this, let me see in the latest version if it fails, I have added the path in the post install config to install it in the correct location. Thanks for pointing it out

hp77-creator avatar Sep 07 '25 15:09 hp77-creator

@RazvanLiviuVarzaru fixed it: https://buildbot.mariadb.org/#/builders/554/builds/14113

hp77-creator avatar Sep 07 '25 16:09 hp77-creator

@FooBarrior shall I rebase all the commits into one for the final merge? All the CI are green now, one that is failing that is not because of my changes and it will pass if its re-triggered again.

hp77-creator avatar Sep 07 '25 20:09 hp77-creator

@hp77-creator yes, please, make a single commit and rebase. Please mention in the commit what is implemented, and what is only tested.

Also mention our approach to reduce the size of the binary (so that there'd be an explanation for the linker flags)

FooBarrior avatar Sep 07 '25 20:09 FooBarrior

@FooBarrior done.

hp77-creator avatar Sep 08 '25 05:09 hp77-creator

Hey @FooBarrior any plans for this PR?

hp77-creator avatar Nov 07 '25 12:11 hp77-creator

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

FooBarrior avatar Nov 07 '25 13:11 FooBarrior