server icon indicating copy to clipboard operation
server copied to clipboard

MDEV-19629: format_bytes implementation

Open 3abqreno opened this issue 11 months ago • 3 comments

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

Description

This PR is for backporting FORMAT_BYTES from MySQL to MariaDB as needed in Jira Issue.

The source of MySQL I backported from is item_pfs_func.h and item_pfs_func.cc

There is still pfs_thread_id and pfs_current_thread_id needs to be backported, I will push them immediately after this is merged

How can this PR be tested?

I have used used MySQL tests too, and it passed the tests successfully too.

Note:

  • This PR is continuing on the work of this pull request after asking Anel and telling to continue his work trying to fix the test errors.
  • I have modified some tests since there is a system function called FORMAT_BYTES, and it gives warning. So I added this warning to all the testcases.

Basing the PR against the correct MariaDB version

  • [x] This is a new feature and the PR is based against the latest MariaDB development 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

3abqreno avatar Mar 18 '24 18:03 3abqreno

Please cherry-pick patch PR #2578 and do your changes and reviews already done in PR on top.

an3l avatar Mar 19 '24 07:03 an3l

can the SQL implementation ins cripts/sys_schema/functions/format_bytes.sql be safely made to proxy though to the new implementation?

The author of the initial PR found out tests were giving out warnings so he started adding warnings in result files like mysql-test/suite/sysschema/r/fn_format_bytes.result but the problem is in checks buildbot/amd64-ubuntu-2204-debug-ps has perfschema disabled so it doesn't have a warning and the tests fail.

in short adding warnings solves the issue in some places and breaks it in others, The original author requested removing sys.format_bytes() but was told to keep it and I need some help to know what do in this case.

3abqreno avatar Mar 19 '24 13:03 3abqreno

Please cherry-pick patch PR #2578 and do your changes and reviews already done in PR on top.

I have already done cherry-pick that PR, as of what I can see the main change needed was making an ancestor class that can handle both format_pico_time and format_bytes().

After reading other comments I was thinking of making a class and call it format_type that has a static function takes an array values and strings to do the matching and in format_bytes/pico_time it will only be calling that function and doing checks first. I did not know where to put that ancestor class exactly and if my idea is the correct implementation of what was asked, if my idea is not correct please guide me.

3abqreno avatar Mar 19 '24 13:03 3abqreno