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

Support createIndex with specifying index prefix length #148

Closed
vaishakhikulkarni opened this issue Sep 30, 2021 · 10 comments · Fixed by #157
Closed

Support createIndex with specifying index prefix length #148

vaishakhikulkarni opened this issue Sep 30, 2021 · 10 comments · Fixed by #157

Comments

@vaishakhikulkarni
Copy link

vaishakhikulkarni commented Sep 30, 2021

In SQL:

ALTER TABLE some_table MODIFY column1 varchar(4096) CHARACTER SET utf8mb4 NOT NULL;
ALTER TABLE some_table ADD INDEX some_table_index (column1 (300),column2);

In Liquibase also we don't support override but have work around like follows:
First createIndex and use modifySql to replace column1 with column1(300) override size.

    <createIndex tableName="some_table" indexName="some_table_index">
        <column name="column1"/>
        <column name="column2"/>
    </createIndex>
    <modifySql dbms="mysql">
        <replace replace="`column1 `" with="`column1 `(300)"/>  <!-- "300" limits index size -->
    </modifySql>

While we do not have any support or work around for liquibase percona. This would be very useful feature for us.

@adangel
Copy link
Collaborator

adangel commented Oct 1, 2021

Do I understand you correctly: You basically want that modifySql is also applied when the ADD INDEX statement is executed via percona? I guess, currently the whole "modifySql" is simply ignored...

Just some docs:

I guess, this should then be supported not only for createIndex but basically for all change types.

@vaishakhikulkarni
Copy link
Author

vaishakhikulkarni commented Oct 1, 2021

No I don't want modifySql to be applied for add Index.
I want a way to override the size of the column while creating a index using createIndex tag as we can do using SQL.
ALTER TABLE some_table MODIFY column1 varchar(4096) CHARACTER SET utf8mb4 NOT NULL;
ALTER TABLE some_table ADD INDEX some_table_index (column1 (300),column2);

In liquibase also we don't have way to override the size of the column while creating index for tag createIndex.
Work around used is to add modifySQL tag.

Because even if we mention type while creating index it gets ignored in liquibase as well as liquibase percona.

    <createIndex tableName="some_table" indexName="some_table_index">
        <column name="column1" type="varchar(300)"/>
        <column name="column2"/>
    </createIndex>

Please let me know accordingly.

@adangel
Copy link
Collaborator

adangel commented Oct 7, 2021

Ok, I understand now.

Have you tried to create the index like this?

    <createIndex tableName="some_table" indexName="some_table_index">
        <column name="column1(300)"/>
        <column name="column2"/>
    </createIndex>

If that works, then all is fine - your use case is already supported. And it should work also with liquibase-percona.

If not (it might be the liquibase properly escapes the column name and you'll see an error like "column column1(300) not found"), then I'd suggest that you bring this into attention for core liquibase. Just file a new feature request here: https://github.com/liquibase/liquibase/issues

For me it makes only sense to implement this feature in liquibase-percona, if it is supported in liquibase-core (e.g. without percona) as well.

The feature we are talking here about is called Column Prefix Key Parts.

@vaishakhikulkarni
Copy link
Author

vaishakhikulkarni commented Oct 7, 2021

So using following code:

<createIndex tableName="some_table" indexName="some_table_index">
        <column name="column1(300)"/>
        <column name="column2"/>
</createIndex>

works with liquibase.

But when we try to use this using liquibase percona we start getting error in Java as
Reason: java.lang.RuntimeException: Percona exited with 139.
The command generated looks like this:

pt-online-schema-change --alter-foreign-keys-method=auto --nocheck-unique-key-change \
    --alter="ADD INDEX some_table_index (`column1(300)`, column2)" --password=*** \
    --execute h=hostName,P=3306,u=admin,D=schmena_name,t= some_table

I also tried to run this command on command line I get following error:

pt-online-schema-change --alter-foreign-keys-method=auto --nocheck-unique-key-change \
    --alter="ADD INDEX some_table_index (`column1(300)`, column2)" --password=*** --**dry-run** \
    h=hostName,P=3306,u=admin,D=schmena_name,t= some_table

Error:
zsh: segmentation fault pt-online-schema-change --alter-foreign-keys-method=auto --dry-run

@vaishakhikulkarni
Copy link
Author

Hi Team,

Any update on this request ?

@adangel adangel changed the title Support createIndex tag to override the column size. Support createIndex with specifying index prefix length Nov 6, 2021
@adangel adangel added this to the next milestone Nov 6, 2021
@adangel
Copy link
Collaborator

adangel commented Nov 6, 2021

I've created a fix for this, see #157.
You can download the fixed liquibase-percona extension here:
liquibase-percona-4.6.2-SNAPSHOT-fixed-148.zip

Would you mind testing this fix?

@mahameed
Copy link

@adangel My colleague @vaishakhikulkarni is out of office these days. I tested your update and it does indeed work now. However, our problem of not be able to change column size while updating index still remains. It seems like the 2 solutions to them would be to:

  • Ask the liquibase team to update the size of the column in a createindex tag instead of having the modifySql
  • Percona starts supporting modifySql tag, especially for createindex

Do you think the 2nd option is something viable?

@adangel
Copy link
Collaborator

adangel commented Nov 15, 2021

@mahameed Thanks for testing.

Regarding your follow-up question: You can specify the column size while creating the index like this:

<createIndex tableName="some_table" indexName="some_table_index">
        <column name="column1(300)"/>
        <column name="column2"/>
</createIndex>

Is there anything else, what should be supported by liquibase? If so, please comment on liquibase/liquibase#2191

So I don't think, modifySql is required at all (at least not for this use case).

@mahameed
Copy link

@adangel ok I missed that part. Yeah that seems to be working. Thanks.
Any rough estimate on when 4.6.2 proper will be released?

adangel added a commit that referenced this issue Nov 19, 2021
Support createIndex with specifying index prefix length (#148) #157

* index-prefix:
  Verify that index prefix length works also without liquibase-percona
  Support createIndex with specifying index prefix length (#148)
@adangel
Copy link
Collaborator

adangel commented Nov 19, 2021

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

Successfully merging a pull request may close this issue.

3 participants