Skip to content
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

Fix #16138 - Ignore the length of the integer types and show a warning #16767

Merged
merged 2 commits into from
Jul 13, 2021
Merged

Fix #16138 - Ignore the length of the integer types and show a warning #16767

merged 2 commits into from
Jul 13, 2021

Conversation

iifawzi
Copy link
Contributor

@iifawzi iifawzi commented Mar 27, 2021 •
edited by williamdes

Signed-off-by: Fawzi E. Abdulfattah iifawzie@gmail.com

Description

  • If the server is MySQL, and the version is 8.0.18 or higher, the length of (SMALLINT, MEDIUMINT, INT, BIGINT,
    or TINYINT with a length not equal to 1 ) will be ignored, I haven't checked whether ZEROFILL attribute is specified or not, since it's deprecated and wouldn't make a difference I think. (ZEROFILL is deprecated and will be removed in MySQL #16742)
  • A warning will be shown as following, whenever any integer type is selected in MySQL with a version >= 8.0.18
    Screen Shot 2021-03-27 at 4 18 51 PM

Fixes #16138

@codecov
Copy link

codecov bot commented Mar 27, 2021 •
edited

Codecov Report

Merging #16767 (e6e496f) into master (be4a577) will decrease coverage by 0.01%.
The diff coverage is 18.18%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #16767      +/-   ##
============================================
- Coverage     45.85%   45.83%   -0.02%     
- Complexity    15262    15271       +9     
============================================
  Files           483      483              
  Lines         60395    60406      +11     
============================================
- Hits          27692    27690       -2     
- Misses        32703    32716      +13
Flag Coverage Δ
arch-7-amd64 48.13% <19.35%> (+0.10%) ⬆️
arch-7-arm32v6 47.98% <19.35%> (-0.10%) ⬇️
arch-7-arm32v7 48.03% <19.35%> (-0.02%) ⬇️
arch-7-arm64v8 48.13% <19.35%> (+0.04%) ⬆️
arch-7-i386 47.98% <19.35%> (+0.01%) ⬆️
arch-7-s390x 47.88% <19.35%> (-0.07%) ⬇️
dbase-extension 45.39% <18.18%> (+<0.01%) ⬆️
recode-extension 45.41% <18.18%> (+0.06%) ⬆️
unit-7.2-ubuntu-latest 45.39% <18.18%> (+0.04%) ⬆️
unit-7.3-ubuntu-latest 48.24% <18.18%> (+0.05%) ⬆️
unit-7.4-ubuntu-latest 48.18% <18.18%> (+0.01%) ⬆️
unit-8.0-ubuntu-latest 48.33% <18.18%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
libraries/classes/Html/Generator.php 17.97% <0.00%> (+0.06%) ⬆️
libraries/classes/Query/Compatibility.php 0.00% <0.00%> (ø)
libraries/classes/Table/ColumnsDefinition.php 0.00% <0.00%> (ø)
libraries/classes/Table.php 43.36% <100.00%> (+0.14%) ⬆️
...ries/classes/Plugins/Auth/AuthenticationConfig.php 73.33% <0.00%> (-5.34%) ⬇️
libraries/classes/Header.php 89.29% <0.00%> (-1.01%) ⬇️
libraries/classes/Transformations.php 60.86% <0.00%> (-0.49%) ⬇️
libraries/classes/Util.php 56.07% <0.00%> (+0.08%) ⬆️
libraries/classes/Charsets.php 85.45% <0.00%> (+3.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be4a577...e6e496f. Read the comment docs.

@iifawzi iifawzi force-pushed the remove-int-length branch 5 times, most recently from 42d10e5 to 8bb6bbf Compare March 27, 2021 15:40
@williamdes williamdes added this to In progress in pull-requests via automation Mar 28, 2021
@williamdes williamdes changed the title FIX #16138 - Ignore the length of the integer types and show a warning Fix #16138 - Ignore the length of the integer types and show a warning Mar 28, 2021
williamdes
williamdes reviewed Apr 13, 2021
View reviewed changes
js/src/functions.js Outdated Show resolved Hide resolved
@Sean4134 Sean4134 mentioned this pull request Apr 14, 2021
Closed
williamdes
williamdes reviewed Apr 15, 2021
View reviewed changes
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work !

templates/columns_definitions/column_definitions_form.twig Outdated Show resolved Hide resolved
libraries/classes/Table.php Outdated Show resolved Hide resolved
williamdes
williamdes approved these changes Apr 15, 2021
View reviewed changes
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Near to perfect 💯

pull-requests automation moved this from In progress to Reviewer approved Apr 15, 2021
@iifawzi
Copy link
Contributor Author

iifawzi commented Apr 15, 2021 •
edited

Near to perfect 💯

I've learned a lot of internals, by working on this! Thanks @williamdes Couldn't be done without your guidance, and thanks for your reviewing time.

williamdes
williamdes approved these changes Apr 15, 2021
View reviewed changes
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯
Deploy time

@williamdes
Copy link
Member

@sudo-robot deploy

@iifawzi iifawzi force-pushed the remove-int-length branch 2 times, most recently from 91c9e91 to 60377e2 Compare July 13, 2021 01:47
@iifawzi
Copy link
Contributor Author

iifawzi commented Jul 13, 2021

Hi, @williamdes, I've fixed the conflicts. I'm just following up with this, is there any other changes required?

@williamdes
Copy link
Member

Hi, @williamdes, I've fixed the conflicts. I'm just following up with this, is there any other changes required?

I think it is time to merge your work, I will rebase/squash your work and test it before merging
Thank you for your patience

iifawzi and others added 2 commits July 13, 2021 15:42
@iifawzi @williamdes
Fix phpmyadmin#16138 - ignore the length of integer types, and show a…
9954c47 
… warning

Fixes: phpmyadmin#16138

Signed-off-by: Fawzi E. Abdulfattah <iifawzie@gmail.com>
@williamdes
Code improvements for Html/Generator::getSupportedDatatypes
e6e496f 
Signed-off-by: William Desportes <williamdes@wdes.fr>
@williamdes
Copy link
Member

@sudo-robot deploy

@williamdes williamdes removed the request for review from MauricioFauth July 13, 2021 18:47
@williamdes williamdes self-assigned this Jul 13, 2021
@williamdes williamdes added enhancement A feature request for improving phpMyAdmin ui Issues relating to the user interface labels Jul 13, 2021
@williamdes williamdes added this to the 5.2.0 milestone Jul 13, 2021
@williamdes williamdes merged commit 0b81db9 into phpmyadmin:master Jul 13, 2021
pull-requests automation moved this from Reviewer approved to Done Jul 13, 2021
@phpmyadmin phpmyadmin deleted a comment from sudo-robot Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williamdes williamdes williamdes approved these changes

Assignees

@williamdes williamdes

Labels
enhancement A feature request for improving phpMyAdmin ui Issues relating to the user interface
Projects
pull-requests
  
Done
Milestone
5.2.0
Development

Successfully merging this pull request may close these issues.

MySQL 8.0.19 Removes the int LENGTH
2 participants
@iifawzi @williamdes