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

MySQL fields with () should not get quoted #162

Open
moltar opened this issue Nov 8, 2023 · 4 comments
Open

MySQL fields with () should not get quoted #162

moltar opened this issue Nov 8, 2023 · 4 comments

Comments

@moltar
Copy link

moltar commented Nov 8, 2023

$sqlt_table->add_index(
  name   => "idx", 
  fields => ["foo(100)"], // size required for text columns in MySQL
);

Related SO question: https://stackoverflow.com/questions/6859955/how-do-you-specify-index-length-when-using-dbixclass


This is from DBIx::Class::Migration:

What I see that in deploy/001-auto.sql it produces invalid SQL:

INDEX `custom_field_map_values_idx_value_text` (`value_text(1000)`),

But in upgrade/001-auto.sql, it looks valid:

ALTER TABLE custom_field_map_values DROP INDEX custom_field_map_values_idx_value_text,
                                    DROP INDEX custom_field_map_values_idx_value_options,
                                    ADD INDEX custom_field_map_values_idx_value_text (value_text(1000)),
                                    ADD INDEX custom_field_map_values_idx_value_options (value_options(1000));
@rabbiveesh
Copy link
Contributor

2 things to note here:

  1. what you're seeing is likely a bug in DBIx::Class::Migration not being consistent in using the quoting options. The easiest solution for the time being is to just turn off quoting
  2. we have an open pr (Add index length option for MySQL #68 ) that addresses this exact problem by allowing you to pass in {name => 'foo', prefix_length => 100} instead of the string. If you have opinions on what that should look like, please comment over there, i do hope to merge it soon

@rabbiveesh
Copy link
Contributor

Hey, this is now released in 1.64

As mentioned in the previous comment, you can use this feature by instead of passing foo(100), you can pass in { name => 'foo', prefix_length => 100 }

@rabbiveesh
Copy link
Contributor

Truth is, the Postgres parser has an escape hatch for when you add parens to a field name, so even though the CORRECT solution to the specific issue here is to use the new interface, we may as well ALSO add in the escape hatch

@rabbiveesh rabbiveesh reopened this Dec 22, 2023
@rabbiveesh rabbiveesh changed the title Invalid and inconsistent handling of indexes with size MySQL fields with () should not get quoted Dec 22, 2023
@moltar
Copy link
Author

moltar commented Dec 22, 2023

@rabbiveesh Thank you! 🙇🏼‍♂️

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

No branches or pull requests

2 participants