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

Moving column with empty default value will replace it with '' #17920

Closed
momala454 opened this issue Nov 24, 2022 · 4 comments
Closed

Moving column with empty default value will replace it with '' #17920

momala454 opened this issue Nov 24, 2022 · 4 comments
Assignees
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Milestone

Comments

@momala454
Copy link

momala454 commented Nov 24, 2022

Describe the bug

When you try to move a column where the default value is an empty string, the generated sql will replace the empty string with ''
Example

`ALTER TABLE `my_table` CHANGE `uniqid` `uniqid` VARCHAR(23) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '\'\'' AFTER `id` `

To Reproduce

Steps to reproduce the behavior:
Create a table with at least 2 columns, one with varchar, and default value empty
change the order of the column
the default value is now ''

Expected behavior

No change in default value

Screenshots

If applicable, add screenshots to help explain the bug.

Server configuration

  • Operating system: linux
  • Web server:
  • Database version: 10.6.11-MariaDB-1:10.6.11+maria~ubu2004
  • PHP version:8.0.25
  • phpMyAdmin version:5.2.0

Client configuration

  • Browser: firefox
  • Operating system: windows 10

Additional context

Add any other context about the bug here.

@williamdes williamdes added Bug A problem or regression with an existing feature affects/5.2 This issue or pull-request affects 5.2.x releases (and maybe further versions) labels Nov 24, 2022
@kamil-tekiela
Copy link
Contributor

kamil-tekiela commented Nov 25, 2022

This was introduced in 75d296a when SHOW CREATE TABLE was replaced with SELECT on INFORMATION_SCHEMA. Setting $cfg['Servers'][$i]['DisableIS'] = true seems to fix the issue.

@williamdes williamdes added affects/6.0 This issue or pull-request affects 6.0.x releases (and maybe further versions) confirmed/5.2 This issue is confirmed to be reproduced on 5.2 at the time this label was set confirmed/6.0 This issue is confirmed to be reproduced on 6.0 at the time this label was set labels Nov 25, 2022
@iifawzi
Copy link
Contributor

iifawzi commented Nov 25, 2022

Hi, this bug is only affecting MariaDB versions newer than 10.2.7, it seems like MariaDB 10.2.7 broke compatibility with MySQL with their INFORMATION_SCHEMA.COLUMNS.COLUMN_DEFAULT new implementation.

There are multiple reports of the issue but it seems to be intentional, not a bug. MDEV-13341.

Most probably we will need to have a specific checks to handle this. 13341#comment

@kamil-tekiela
Copy link
Contributor

Ok, so if I understand correctly, we have the following three points:

  • We get different behaviour on MariaDB 10.2.7 vs all previous versions and MySQL. MySQL might fix this bug soon too.
  • We have different output from IS and from SHOW COLUMNS
  • If we stick with the new behaviour of IS then we have quoted and properly formatted value coming from IS vs what we get from HTML form. Thus we cannot use the same logic to build ALTER TABLE. We'd probably need some flag that the value is formatted. e.g. for default value a'b"c\d\n we get 'a''b"c\\d\\n' in MariaDB 10.2.7+, while in MySQL we get a'b"c\d\n.

I would propose removing IS SELECTs and sticking with SHOW COLUMNS, but as I understand it, we would be bringing the bug back. It was a bug fix after all. But... phpMyAdmin doesn't support default expressions. If you try to use them, then you will get all sorts of errors. So technically, the bug doesn't affect phpMyAdmin that much yet, just like it was unnoticeable in previous MariaDB versions and MySQL.

Does anyone have any ideas how to fix this?

@iifawzi
Copy link
Contributor

iifawzi commented Nov 26, 2022

I believe that we need to keep the use of IS, we're definitely going to support the default expressions in the future, so we will still need the information we get from IS at that time.

The issue is only in the retrieval, not the insertion. MariaDB will always quote the values if they're not expressions, so I do suggest having a helper maybe getMariaDBDefaultValue, that gets the actual values without the quotations, or return it as it if we detected that this's an expression, somehow.

@williamdes williamdes added has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete and removed affects/5.2 This issue or pull-request affects 5.2.x releases (and maybe further versions) affects/6.0 This issue or pull-request affects 6.0.x releases (and maybe further versions) confirmed/5.2 This issue is confirmed to be reproduced on 5.2 at the time this label was set confirmed/6.0 This issue is confirmed to be reproduced on 6.0 at the time this label was set labels Apr 28, 2024
@williamdes williamdes self-assigned this Apr 28, 2024
@williamdes williamdes added this to the 5.2.2 milestone Apr 28, 2024
williamdes added a commit that referenced this issue Apr 28, 2024
Signed-off-by: William Desportes <williamdes@wdes.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Projects
None yet
Development

No branches or pull requests

4 participants