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

Column options are ignored for GENERATED / VIRTUAL / STORED columns #13854

Closed
mvorisek opened this issue Dec 6, 2017 · 19 comments
Closed

Column options are ignored for GENERATED / VIRTUAL / STORED columns #13854

mvorisek opened this issue Dec 6, 2017 · 19 comments
Assignees
Labels
Bug A problem or regression with an existing feature
Projects
Milestone

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Dec 6, 2017

Steps to reproduce

  1. create or update any GENERATED / VIRTUAL / STORED column with "NOT NULL", specifict charset or type options like "UNSINGED"
  2. save

Example

Create table with one STORED column with type VARCHAR(10), collation ascii_general_ci and NOT NULL. This is currently not possible via GUI. The GUI currently also ignores these options on column def. change, resulting that all options are removed.

Expected behaviour

Configured column options should be saved as normal (not generated) columns. Columns options are fully supported by MySQL and honored / validated as for normal columns.

Actual behaviour

Column options are ignored in columns create or update via GUI.

Server configuration

any

@mvorisek mvorisek changed the title Column options and GENERATED / VIRTUAL / STORED columns Column options are ignored for GENERATED / VIRTUAL / STORED columns Dec 7, 2017
@mvorisek mvorisek changed the title Column options are ignored for GENERATED / VIRTUAL / STORED columns BUG: Column options are ignored for GENERATED / VIRTUAL / STORED columns Dec 14, 2017
@mvorisek mvorisek changed the title BUG: Column options are ignored for GENERATED / VIRTUAL / STORED columns Column options are ignored for GENERATED / VIRTUAL / STORED columns Dec 18, 2017
@williamdes
Copy link
Member

Can you try with version 4.8.1 ?
I am unable to reproduce this issue.

@mvorisek
Copy link
Contributor Author

mvorisek commented Jun 10, 2018

Issue evaluated in 4.8.1:

Create table:

CREATE TABLE ttt (
  test int(11) NOT NULL,
  testvirt varchar(11) COLLATE utf8_bin GENERATED ALWAYS AS (test) VIRTUAL
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin;

Go to the "Structure" tab and try to change the collation of the generated column to "utf8_general_ci".

Currently the PhpMyAdmin generates:

ALTER TABLE ttt CHANGE testvirt testvirt VARCHAR(11) AS (test) VIRTUAL; 

But it should generate something like this:

ALTER TABLE ttt CHANGE testvirt testvirt VARCHAR(11) CHARACTER SET utf8
    COLLATE utf8_general_ci AS (test) VIRTUAL NULL;

@williamdes
Copy link
Member

williamdes commented Jun 10, 2018

MariaDB does not support COLLATE utf8_bin in CREATE statement.
And the query the PMA should generate does not work in MariaDB.

Maybe this explains why this issue.

@mvorisek
Copy link
Contributor Author

Yes, tested on MySQL. Maria added the virtual columns support sooner and the syntax and features differs.

Here is my proposal for MySQL:
d254e8a

Currently is GUI at least for me is unsuable as I need to edit the virtual column like the normal columns. The PhpMyAdmin can generate the full ALTER parameters (collation, nullable, etc.) at least for MySQL. I have no experience with Maria DB as I use MySQL only.

@williamdes williamdes added Bug A problem or regression with an existing feature help wanted labels Nov 28, 2018
@williamdes williamdes added this to To be sorted in issues May 2, 2019
@williamdes
Copy link
Member

williamdes commented May 4, 2019

@mvorisek Does this issue still exist (on 4.8.6-dev) ?

@williamdes williamdes moved this from To be sorted to n/a priority in issues May 4, 2019
@mvorisek
Copy link
Contributor Author

The issue is still presented and once virtual/generated column is changed in the edit, all the column details like collation are discarded,

@williamdes
Copy link
Member

https://mariadb.atlassian.net/browse/MDEV-7655

CREATE TABLE `vcol_test` (
       `v_col1` varchar(255) COLLATE latin1_general_ci AS (col1) VIRTUAL,
       `col1` varchar(50) COLLATE latin1_general_ci DEFAULT NULL
     ) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_general_ci;

works on MySQL 8.0 and not on MariaDB
Seems to have an issue with COLLATE and CHARSET

So this issue can no be solved in a good way

@williamdes williamdes added waiting on upstream Issues blocked by a third-party and removed help wanted labels May 15, 2019
@williamdes
Copy link
Member

williamdes commented May 15, 2019

Does not work:

ALTER TABLE `contact` CHANGE `zdzd1` `zdzd1` VARCHAR(22) COLLATE utf8_slovenian_ci  AS ( 1 + 1 ) VIRTUAL
ALTER TABLE `contact` CHANGE `zdzd1` `zdzd1` VARCHAR(22)  CHARACTER SET utf8 COLLATE utf8_slovenian_ci  AS ( 1 + 1 ) VIRTUAL

Works:

ALTER TABLE `contact` CHANGE `zdzd1` `zdzd1` VARCHAR(22) CHARACTER SET utf8 AS ( 1 + 1 ) VIRTUAL

@mvorisek
Copy link
Contributor Author

MySQL handles the virtual/generated columns differently and the the PhpMyAdmin needs to do the same.

I will update the patch with Util::getServerType() === 'MySQL' condition tomorrow.

@mvorisek
Copy link
Contributor Author

Generator fixed in PR #15275

Mysql: fixed
MariadDB: not affected

Can the PR be merged and included in the next release?

@williamdes
Copy link
Member

@mvorisek Yes, I think so
Is the PR finished?

@williamdes williamdes self-assigned this May 17, 2019
@mvorisek
Copy link
Contributor Author

Yes. PR is complete. Extra attributes for MySQL are fixed, non-virtual columns or columns in other databases are untouched.

@williamdes
Copy link
Member

Hi @mvorisek I would like to merge the PR, can you implement a check to disable collation change since it does not change for MariaDB.
Actually changing the collation creates a modal (who cares) in javascript but creates an error since we try to change the collation (and we did say in the preview SQL that it would not change)

@mvorisek
Copy link
Contributor Author

@williamdes Please describe how to replicate.

  • I tried to change collation for non-virtual string column with Maria DB - as normal, it works.
  • I tried to change collation for virtual string column with Maria DB - it seems Maria DB does not accept any collation nor nullable info and the PR handles it correctly - it does not include these details in the generated SQL field.

@williamdes
Copy link
Member

@mvidner To reproduce go to my demo server 4.8.6 (public/public)

  • Run query on "MariaDB 3 (cookie)" server
CREATE TABLE `vcol_test11` (
       `v_col1` varchar(255)  AS (col1) VIRTUAL,
       `col1` varchar(50) COLLATE latin1_general_ci DEFAULT NULL
     ) ENGINE=InnoDB DEFAULT CHARSET=latin1 COLLATE=latin1_general_ci;
  • Go to structure and try to change the collation of v_col1
  • Click preview > NO collation (as expected)
  • Close preview and click "Save"
  • Accept modal
  • See error
    image

@mvorisek
Copy link
Contributor Author

The same for MySQL actually. As you have written, the preview SQL it fine (and can be sucessfully commited for both MariaDB and Mysql).

The "save" generates ALTER TABLE `vcol_test11` CHANGE `v_col1` `v_col1` BLOB;. Notice the BLOB at the end even if submitted in the GUI as VARCHAR. It seems to be some issue with PhpMyAdmin and not directly related with this patch. Any idea why the SQL differs with the preview?

@williamdes
Copy link
Member

@mvorisek Yes, there is an additional query that is added if the charset changed between the initial value of the form and the real value that is submitted

The preview is always not accurate since this query is always missing.

@williamdes williamdes removed the waiting on upstream Issues blocked by a third-party label May 19, 2019
@williamdes williamdes added this to the 4.8.6 milestone May 19, 2019
williamdes added a commit that referenced this issue May 19, 2019
Signed-off-by: William Desportes <williamdes@wdes.fr>
issues automation moved this from n/a priority to Closed May 19, 2019
@mvorisek
Copy link
Contributor Author

@williamdes Great work!

@williamdes
Copy link
Member

@mvorisek Thanks to you !

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A problem or regression with an existing feature
Projects
issues
  
Closed
Development

No branches or pull requests

2 participants