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

Add support for creating, altering, and dropping indices #167

Merged
merged 17 commits into from Apr 30, 2024

Conversation

nikochiko
Copy link
Contributor

@nikochiko nikochiko commented Aug 31, 2023

Right now, creating indices is only being done automatically from the CREATE TABLE statements. This pull request aims to change that and allow adding indices to existing tables.

  • Optional support for ALTER INDEX (maybe configurable?)

Parser changes:

  • Support for functional indices: is_functional flag in key_dic
  • Support for partial indices: is_partial flag in key_dic
  • Support for fulltext or spatial indices: is_fulltext or is_partial flag in key_dic
  • Support for detecting index type: index_type in key_dic as either BTREE or HASH.
  • Support CREATE INDEX
  • Support ALTER TABLE ADD INDEX
  • Support ALTER TABLE RENAME INDEX
  • Support ALTER TABLE DROP INDEX
  • Support DROP INDEX

Open questions

  • What should the default for index_type be? I'm setting it to None for now.

@nikochiko nikochiko changed the title Add support for creating and altering indices Add support for creating, altering, and dropping indices Aug 31, 2023
@nikochiko nikochiko changed the base branch from main to gsoc-2023 August 31, 2023 22:37
@nikochiko
Copy link
Contributor Author

nikochiko commented Aug 31, 2023

@the4thdoctor I accidentally carried over an older change in the pg_lib.py module, but it's relevant here because it adds support for CREATE INDEX statement to pg_engine.
Can you take a look at it and let me know if it's okay to be kept in?

This in particular:

https://github.com/the4thdoctor/pg_chameleon/pull/167/files#diff-90a4962e0a4cdb4a234ce24051bef373497075488d48cb05e3f87eac35917fbdR1351-R1353

Edit: Removed that change entirely because we needed to rename that command from CREATE INDEX to ADD INDEX subcommand of ALTER TABLE

@nikochiko
Copy link
Contributor Author

nikochiko commented Aug 31, 2023

Support for CREATE INDEX and ALTER TABLE ... ADD INDEX is added.

One open issue there is that the index_name is not being captured in the case of ALTER TABLE ... ADD INDEX.

That's because by default, even when parsing indices in CREATE TABLE, the index name is not captured and the index_name key instead tells about the kind of index it is: e.g. PRIMARY, UNIQUE, OTHER, INDEX, etc.

I'm not sure if changing one of those will also break backward compatibility with the other modules such as pg_engine or mysql_engine.

@nikochiko
Copy link
Contributor Author

@the4thdoctor we had discussed the ordering for subcommands in alter table statements, that additions of columns should come before additions of indices, and removing indices should come before removing columns. But that would have some edge cases, e.g.

ALTER TABLE table_name
    DROP COLUMN column1,
    RENAME COLUMN column2 TO column1,
    ADD COLUMN column2 VARCHAR(50);

If we change the order here and add column2 first, that will throw an error because it already exists.

Can we use the same order in which the commands appeared in the MySQL query? That should already be accurate, I think.

@the4thdoctor
Copy link
Owner

@the4thdoctor we had discussed the ordering for subcommands in alter table statements, that additions of columns should come before additions of indices, and removing indices should come before removing columns. But that would have some edge cases, e.g.

ALTER TABLE table_name
    DROP COLUMN column1,
    RENAME COLUMN column2 TO column1,
    ADD COLUMN column2 VARCHAR(50);

If we change the order here and add column2 first, that will throw an error because it already exists.

Can we use the same order in which the commands appeared in the MySQL query? That should already be accurate, I think.

If you can manage easily this particular case please go for it. Thanks.

@nikochiko
Copy link
Contributor Author

This is already taken care of. We use the same order as the MySQL commands we receive.

@the4thdoctor
Copy link
Owner

Thanks and sorry for bein MIA for a while.
I'll test today or tomorrow then I'll merge the PR and will make a long overdue release with your contribution.

@the4thdoctor
Copy link
Owner

Apologies for the long delay, I've checked the PR and the previous tests are running fine.
I'm merging it.
Ta!

@the4thdoctor the4thdoctor merged commit c3b2163 into the4thdoctor:gsoc-2023 Apr 30, 2024
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

Successfully merging this pull request may close these issues.

None yet

2 participants