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

Restore index, foreign key and sequence constraint names #20

Open
shayonj opened this issue Feb 5, 2022 · 9 comments
Open

Restore index, foreign key and sequence constraint names #20

shayonj opened this issue Feb 5, 2022 · 9 comments
Labels
enhancement New feature or request p2 post-launch

Comments

@shayonj
Copy link
Owner

shayonj commented Feb 5, 2022

The naming can get weird or look weird when you pgosc has run multiple times on a table. It will always prefix the the prefix_key and additional run into name length limit issues.

This also applies to sequence names.

Can happen as part of cleanup.

@shayonj shayonj added p2 enhancement New feature or request labels Feb 5, 2022
@shayonj shayonj changed the title Restore index and foreign key constraint names Restore index, foreign key and sequence constraint names Jul 4, 2022
@sasharevzin
Copy link

I would like to express my heartfelt gratitude to you for your unwavering commitment to maintaining this invaluable utility. Your dedication to ensuring its continued functionality has not gone unnoticed, and I am truly appreciative of the effort you invest in this endeavor.

With the utmost respect and appreciation, I kindly inquire if there have been any recent developments or updates with regard to the previously closed pull request (PR). Your guidance and insights on this matter would be greatly valued, as your expertise plays a pivotal role in shaping the utility's performance and efficiency.

Once again, thank you ever so much for your tireless work in maintaining this utility, and I eagerly await any information you can share regarding the aforementioned PR. Your contributions are instrumental in enhancing our workflow, and your commitment to excellence is truly commendable.

@shayonj
Copy link
Owner Author

shayonj commented Sep 15, 2023

hey hey! Thanks for the message. I haven't thought about this much lately. Let me thinking about this weekend and get back. I agree, it'd be really nice to support this functionality.

@shayonj
Copy link
Owner Author

shayonj commented Sep 17, 2023

So, given that we use including all to copy the original schema structure and indexes here, PG itself chooses and ensures that the indexes are unique across table names. And, so the index name ends being pgosc_st_$TABLE_NAME_$ID_$COLUMN_key. This is because the name of the new/shadow table itself is pgosc_st_$TABLE_NAME_$ID.

I think restoring to the original index name is going to be hard because the low level id (OID) of the table changes too (since we fork a new table and default to the new table as the primary table at the end of the swap!).

However, we could at least remove pgosc_ bits from the name and make it so it is $TABLE_NAME_$COLUMN_key. So, pgosc_st_books_123abc_last_login_key -> books_last_login_key.

As noted above, we will lose the original naming, but at least the name isn't littered with the shadow table naming convention. We can do the same for sequences too.

Curious if folks have thoughts here (cc @jfrost). Tagging explicitly based on the emoji reaction

@brycethornton @sasharevzin @Shohou @sasharevzin

@jfrost
Copy link
Collaborator

jfrost commented Sep 18, 2023

We could hang onto the index names from the original table and then rename the new indexes after table drop if --drop is passed. Otherwise, I guess we could rename the original table's indexes after swap, then rename the new table's indexes to match the original names. Renaming indexes requires a SHARE UPDATE EXCLUSIVE lock which should be relatively safe. You can see more info here: https://www.postgresql.org/docs/current/explicit-locking.html#LOCKING-TABLES .

@brycethornton
Copy link
Contributor

I'd vote for renaming the original table's indexes after swap, then rename the new table's indexes to match the original names, as @jfrost mentioned. This would work even if the user does pass the --drop flag. Is an index rename a fast operation even on a very large table?

@jfrost
Copy link
Collaborator

jfrost commented Sep 18, 2023

Is an index rename a fast operation even on a very large table?
They are generally a few milliseconds as long as you don't run afoul of a locking issue. We can utilize lock_timeout with exception catching for extra safety.

@shayonj
Copy link
Owner Author

shayonj commented Sep 18, 2023

I think the tricky bit is creating a map of old index names and the new index names. Since a column can have multiple indexes, and a user could've use custom naming for index, I wonder if there is a reliable way of mapping a column (or a composite/set of columns) against an index name. And accordingly derive indexes based on column names, so we can accordingly rename post swap.

Let me know in case it doesn't make sense. I will give it a bit more thought later and get back.

@jfrost
Copy link
Collaborator

jfrost commented Sep 20, 2023

@shayonj I'm not sure what you're asking exactly. We have all the index names and index definitions for each table available in pg_indexes. https://www.postgresql.org/docs/current/view-pg-indexes.html

We're already grabbing that info here:

def get_indexes_for(client, table)
query = <<~SQL
SELECT indexdef, schemaname
FROM pg_indexes
WHERE schemaname = '#{client.schema}' AND tablename = '#{table}'
SQL
indexes = []
run(client.connection, query) { |result| indexes = result.map { |row| row["indexdef"] } }
indexes
end

Are you thinking that we need to know the column names in the index in order to rename them?

We don't, we can just cache the index names and then rename them all with something like:

ALTER INDEX foo_bar_idx RENAME TO foo_bar_idx_old;

That's obviously oversimplified, but it seems doable at first glance.

@shayonj
Copy link
Owner Author

shayonj commented Sep 30, 2023

Sorry for the late reply - Yeah i was talking about this line:

'CREATE TABLE %s (LIKE %s including all) WITH (autovacuum_enabled = false)',

Basically, when we create the shadow table with including all, we lose the original naming, so it will now be hard to map them.

But yes, I think what we can do is take control of the index building/adding process and that way we can build the in-memory map of the old index names and rename them again post cutover. Which should work like you mentioned.

Its a bit of work, but I think def achievable for indexes.

We will then have to do the same for sequences (right now we "fix" it here:

FUNC_FIX_SERIAL_SEQUENCE = <<~SQL
).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2 post-launch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants