Fix pk sequence value update when negative pk values are in use#51
Merged
timb07 merged 5 commits intoSep 9, 2025
Merged
Conversation
Coverage Report Results
1 empty file skipped. |
timb07
force-pushed
the
fix-pk-sequence-value-update-negative
branch
from
d942807 to
faa4e62
Compare
timb07
force-pushed
the
disallow-deferrable-unique-constraint
branch
2 times, most recently
from
a589f56 to
3d22d07
Compare
timb07
force-pushed
the
fix-pk-sequence-value-update-negative
branch
from
faa4e62 to
b6346a5
Compare
timb07
requested review from
felcury and
marcelofern
and removed request for
felcury
marcelofern
approved these changes
Sep 8, 2025
Collaborator
marcelofern
left a comment
marcelofern
left a comment
There was a problem hiding this comment.
Looks good. One comment for consideration
Comment on lines
-1402
to
-1410
| @pytest.mark.xfail( | ||
| raises=NumericValueOutOfRange, | ||
| reason=""" | ||
| E psycopg.errors.NumericValueOutOfRange: integer out of range | ||
| E CONTEXT: SQL function "psycopack_repacked_6723301_fun" statement 1 | ||
| E SQL statement "SELECT "public"."psycopack_repacked_6723301_fun"(NEW."id", NEW."id")" | ||
| E PL/pgSQL function psycopack_repacked_6723301_tgr() line 4 at PERFORM | ||
| """, | ||
| ) |
Collaborator
There was a problem hiding this comment.
I'm wondering what happens here for other different types of primary keys and if the code handles them. I.e., if we parametrise the test like this:
@pytest.mark.parametrize(
"initial_pk_type",
(
"integer",
"serial",
"smallint",
"smallserial",
),
)Thoughts?
Contributor
Author
There was a problem hiding this comment.
I added another commit to cover the above. With slight adjustment to a test factory, the code handles all these PK types.
timb07
force-pushed
the
disallow-deferrable-unique-constraint
branch
from
3d22d07 to
f7314da
Compare
…nstraint Disallow deferrable unique constraints
timb07
force-pushed
the
fix-pk-sequence-value-update-negative
branch
from
5384935 to
652f9e0
Compare
This was referenced Sep 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If the table is being converted to a bigint pk and the pk sequence has been set to negative, we want to reset the sequence value to 2**31 so that it can start using the new bigint range (and not run out of negative values when the sequence advances to 1).
Previously that reset was done in the
swapstage. But that led to errors, since the old table was being updated by a trigger, and it only had an integer pk.This change moves the sequence reset to the
clean_upstage after the trigger has been dropped, which fixes the bug. The change also ensures that the sequence on the new table has the same min value as the old sequence (which could be negative).Fixes #49