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

Migration fails when sequence name is too long #1198

Closed
andrewcfrancis2 opened this issue Jan 14, 2020 · 10 comments
Closed

Migration fails when sequence name is too long #1198

andrewcfrancis2 opened this issue Jan 14, 2020 · 10 comments

Comments

@andrewcfrancis2
Copy link

I upgraded from EFCore 2.1 to 3.1 and my next migration wants to convert all my identity columns from Serial to IdentityByDefault. I've already run into issue #1157 so I downgraded Npgsql to 3.0.1.

Now this is a separate issue. I have long table and column names and the sequence names are more than 63 chars. The migration generates this SQL script:

ALTER TABLE trust_account_transaction_files_history ALTER COLUMN trust_account_transaction_file_history_id TYPE integer;
ALTER TABLE trust_account_transaction_files_history ALTER COLUMN trust_account_transaction_file_history_id SET NOT NULL;
ALTER SEQUENCE trust_account_transaction_files_history_trust_account_transaction_file_history_id_seq RENAME TO trust_account_transaction_files_history_trust_account_transaction_file_history_id_old_seq;
ALTER TABLE trust_account_transaction_files_history ALTER COLUMN trust_account_transaction_file_history_id DROP DEFAULT;
ALTER TABLE trust_account_transaction_files_history ALTER COLUMN trust_account_transaction_file_history_id ADD GENERATED BY DEFAULT AS IDENTITY;
SELECT * FROM setval('trust_account_transaction_files_history_trust_account_transaction_file_history_id_seq', nextval('trust_account_transaction_files_history_trust_account_transaction_file_history_id_old_seq'), false);
DROP SEQUENCE trust_account_transaction_files_history_trust_account_transaction_file_history_id_old_seq;

The ALTER SEQUENCE command fails and I think it's because both old and new names are truncated automatically by Postgres to the same name.

Additionally, it seems like Npgsql 3.x deals with sequence names differently. Version 2.1 truncated both table and column name to create a sequence called trust_account_transaction_fil_trust_account_transaction_fil_seq or even trust_account_transaction_fil_trust_account_transaction_fi_seq1 is there's a conflict. In version 3.x, it just uses the long name and Postgres automatically truncates it removing the _seq suffix.

@roji
Copy link
Member

roji commented Jan 14, 2020

@bricelam does this ring any bells?

@bricelam
Copy link
Contributor

@AndriySvyryd might remember any recent changes to truncation and uniquification.

@AndriySvyryd
Copy link

I don't recall any intentional changes to this. Also I don't recall even having any truncation logic for sequence names.

@andrewcfrancis2
Copy link
Author

Regarding the truncation of the table and column names, I now think that's not in Npgsql but in Postgres itself. I'm still working on that migration and the script that's generated includes this snippet:

ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id TYPE integer;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id SET NOT NULL;
ALTER SEQUENCE trust_account_transactions_history_trust_account_transaction_history_id_seq RENAME TO trust_account_transactions_history_trust_account_transa_old_seq;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id DROP DEFAULT;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id ADD GENERATED BY DEFAULT AS IDENTITY;
SELECT * FROM setval('trust_account_transactions_history_trust_account_transaction_history_id_seq', nextval('trust_account_transactions_history_trust_account_transa_old_seq'), false);
DROP SEQUENCE trust_account_transactions_history_trust_account_transa_old_seq;

Notice that I had to edit the script to shorten the name of the temporary sequence (3rd line). Even doing that, it fails to run in Postgres on the SELECT* FROM ... line because it can't find the first name. Executing this statement:

select pg_get_serial_sequence('trust_account_transactions_history', 'trust_account_transaction_history_id')

Results in "public.trust_account_transactions_hi_trust_account_transaction_his_seq".

I think that means that Postgres itself created the truncated name when the column was converted to GENERATED BY DEFAULT AS IDENTITY. Unfortunately, that's another place where this migration will fail.

@YohDeadfall
Copy link
Contributor

From 4.1.1. Identifiers and Key Words:

The system uses no more than NAMEDATALEN-1 bytes of an identifier; longer names can be written in commands, but they will be truncated. By default, NAMEDATALEN is 64 so the maximum identifier length is 63 bytes. If this limit is problematic, it can be raised by changing the NAMEDATALEN constant in src/include/pg_config_manual.h

@AndriySvyryd
Copy link

Filed dotnet/efcore#19608

@roji
Copy link
Member

roji commented Jan 16, 2020

@andrewcfrancis2 @YohDeadfall the truncation is indeed being done by PostgreSQL, but EF Core has a facility precisely for "uniquifying" in the face of such truncation. In other words, Npgsql informs EF Core about the 63 character limit, and EF Core automatically truncates identifiers and adds ~1, ~2 where necessary.

The issue here is that this uniquification isn't done for sequence names, only for tables/columns/etc.

@roji
Copy link
Member

roji commented Jan 16, 2020

Am going to close this - dotnet/efcore#19608 tracks this on the EF Core side.

@roji roji closed this as completed Jan 16, 2020
@andrewcfrancis2
Copy link
Author

@roji it sounds like you have a handle on it so thanks for being so responsive.

For reference, the auto-generated migration produced code like this:

ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id TYPE integer;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id SET NOT NULL;
ALTER SEQUENCE trust_account_transactions_history_trust_account_transaction_history_id_seq RENAME TO trust_account_transactions_history_trust_account_transaction_history_id_old_seq;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id DROP DEFAULT;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id ADD GENERATED BY DEFAULT AS IDENTITY;
SELECT * FROM setval('trust_account_transactions_history_trust_account_transaction_history_id_seq', nextval('trust_account_transactions_history_trust_account_transaction_history_id_old_seq'), false);
DROP SEQUENCE trust_account_transactions_history_trust_account_transaction_history_id_old_seq;

The problems I encountered were:

  • The first identifier in the ALTER SEQUENCE statement is incorrect so it can't find the sequence. The correct identifier is trust_account_transactions_hi_trust_account_transaction_his_seq
  • Both identifiers in the ALTER SEQUENCE statement are truncated to the same value so the rename fails
  • The first identifier in the SELECT * ... statement is incorrect, the same as the first item. Here I was able to use pg_get_serial_sequence() to get the correct name but I wasn't able to get it to work in the ALTER SEQUENCE statement

In summary, I was able to edit the migration script to run successfully like this:

ALTER SEQUENCE trust_account_transactions_hi_trust_account_transaction_his_seq RENAME TO trust_account_transactions_history_trust_account_transaction_history_id_seq;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id TYPE integer;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id SET NOT NULL;
ALTER SEQUENCE trust_account_transactions_history_trust_account_transaction_history_id_seq RENAME TO trust_account_transactions_history_trust_account_transa_old_seq;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id DROP DEFAULT;
ALTER TABLE trust_account_transactions_history ALTER COLUMN trust_account_transaction_history_id ADD GENERATED BY DEFAULT AS IDENTITY;
SELECT * FROM setval(pg_get_serial_sequence('trust_account_transactions_history', 'trust_account_transaction_history_id'), nextval('trust_account_transactions_history_trust_account_transa_old_seq'), false);
DROP SEQUENCE trust_account_transactions_history_trust_account_transa_old_seq;

I hope that helps!

@roji
Copy link
Member

roji commented Jan 16, 2020

@andrewcfrancis2 thanks! Seems like you managed to work around the problem quite successfully. The actual issue will hopefully be fixed in EF Core itself soon.

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

5 participants