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.

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

Fix #18962 - Moving columns with ENUM() datatype and DEFAULT value causes invalid SQL #19009

Closed

Conversation

johnson361
Copy link
Contributor

@johnson361 johnson361 commented Feb 24, 2024

Description

 SELECT
    `COLUMN_NAME` AS `Field`,
    `COLUMN_TYPE` AS `Type`,
    `COLLATION_NAME` AS `Collation`,
    `IS_NULLABLE` AS `Null`,
    `COLUMN_KEY` AS `Key`,
    `COLUMN_DEFAULT` AS `Default`,
    `EXTRA` AS `Extra`,
    `PRIVILEGES` AS `Privileges`,
    `COLUMN_COMMENT` AS `Comment`
FROM
    `information_schema`.`COLUMNS`  where `TABLE_SCHEMA` = 'test'  AND `TABLE_NAME` = 'test' ;;

When executing this query on MariaDB, the COLUMN_DEFAULT column returns default values enclosed in single quotes on both sides, causing an issue. However, in MySQL, this behavior is not observed. To address the inconsistency, I needed to trim off any extra single quotes that might exist in the COLUMN_DEFAULT values using TRIM('\'' FROM COLUMN_DEFAULT`). Now, the issue is resolved.

Fixes #18962

You can see how the data appeared before the query modifications in the image below.
[
Issue
]

@johnson361
Copy link
Contributor Author

Can anyone please review my PR ?

@ChrisHSandN
Copy link

If you trim ' off then this will fail if the default value starts or ends with an ' e.g.

CREATE TABLE `test` (
  `id` int(11) NOT NULL,
  `enum` enum('yes','no','''foo') COLLATE utf8_unicode_ci NOT NULL DEFAULT '''foo',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci

Can it use the same logic as the Structure tab as this looks to work correctly even if there is a leading ':
image

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 48.61%. Comparing base (4eb4a1a) to head (f07285e).
Report is 13 commits behind head on QA_5_2.

Files Patch % Lines
...trollers/Table/Structure/MoveColumnsController.php 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             QA_5_2   #19009   +/-   ##
=========================================
  Coverage     48.61%   48.61%           
- Complexity    17006    17027   +21     
=========================================
  Files           607      607           
  Lines         72318    72366   +48     
=========================================
+ Hits          35154    35182   +28     
- Misses        37164    37184   +20     
Flag Coverage Δ
dbase-extension 48.21% <0.00%> (+0.04%) ⬆️
recode-extension 48.14% <0.00%> (-1.14%) ⬇️
unit-7.2-ubuntu-latest 48.59% <0.00%> (+0.51%) ⬆️
unit-7.3-ubuntu-latest 48.54% <0.00%> (-0.76%) ⬇️
unit-7.4-ubuntu-latest 49.87% <0.00%> (+1.37%) ⬆️
unit-8.0-ubuntu-latest 48.57% <0.00%> (+0.02%) ⬆️
unit-8.1-ubuntu-latest 48.58% <0.00%> (-0.73%) ⬇️
unit-8.2-ubuntu-latest 48.50% <0.00%> (-0.06%) ⬇️
unit-8.3-ubuntu-latest 49.83% <0.00%> (+1.27%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@johnson361
Copy link
Contributor Author

@ChrisHSandN i have updated the logic . Please retest. Thank you .

yarn.lock Outdated Show resolved Hide resolved
@johnson361 johnson361 force-pushed the move_columns_enum_default_18962 branch 3 times, most recently from b84cf8f to 5f92a81 Compare March 1, 2024 14:12
Signed-off-by: Robert Johnson Nallori <johnson361@gmail.com>
@johnson361 johnson361 force-pushed the move_columns_enum_default_18962 branch from 5f92a81 to fe5af1a Compare March 2, 2024 04:25
@williamdes williamdes requested review from MauricioFauth and removed request for kamil-tekiela March 5, 2024 20:40
@williamdes williamdes changed the title Fix issue: Moving columns with ENUM() datatype and DEFAULT value causes invalid SQL 18962 Fix #18962 - Moving columns with ENUM() datatype and DEFAULT value causes invalid SQL Mar 5, 2024
Signed-off-by: Robert Johnson Nallori <johnson361@gmail.com>
@williamdes
Copy link
Member

Closing this one since #19105 got merged

Thank you for contributing to fix this issue. Please test with the new changes and let us know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants