Skip to content

Conversation

@timb07
Copy link
Contributor

@timb07 timb07 commented Jun 17, 2025

Prior to this change, if the table being repacked used negative pk values, the backfill would cover the entire range from min(pk) to 0 (as well as the positive values). When negative pk values have been used after positive values are nearing exhaustion, it's likely that most of the negative range is empty and therefore does not need to be backfilled.

This change checks for min(pk) and max(pk) separately for positive values and negative values, to avoid backfilling empty negative ranges.

@github-actions
Copy link

Coverage Report Results

Name Stmts Miss Branch BrPart Cover
src/psycopack/__init__.py 6 0 0 0 100%
src/psycopack/_commands.py 109 0 8 0 100%
src/psycopack/_conn.py 5 0 0 0 100%
src/psycopack/_const.py 3 0 0 0 100%
src/psycopack/_cur.py 24 0 2 0 100%
src/psycopack/_identifiers.py 12 0 2 0 100%
src/psycopack/_introspect.py 160 0 16 0 100%
src/psycopack/_logging.py 2 0 0 0 100%
src/psycopack/_psycopg.py 5 0 0 0 100%
src/psycopack/_registry.py 58 0 6 0 100%
src/psycopack/_repack.py 353 0 118 0 100%
src/psycopack/_tracker.py 165 0 36 0 100%
tests/conftest.py 19 0 0 0 100%
tests/factories.py 40 0 8 0 100%
tests/test_cur.py 20 0 2 0 100%
tests/test_fixtures.py 5 0 0 0 100%
tests/test_package.py 3 0 0 0 100%
tests/test_repack.py 702 0 0 0 100%
TOTAL 1691 0 198 0 100%

1 empty file skipped.

@timb07 timb07 force-pushed the backfill-positive-negative branch from 45b8a95 to ee0668b Compare June 18, 2025 00:06
Copy link
Collaborator

@marcelofern marcelofern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Worth noting though that the get_min_and_max_pk() function only gets used by the _populate_backfill_log, so we could return negative/positive ranges directly from it to avoid a couple of round-trips to the db. But not a big deal here.

Comment on lines 238 to 239
INSERT INTO {table_name} ("id")
SELECT "gs" FROM generate_series(1, {positive_rows}) AS gs;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍷 You should be able to simplify the "gs" bits off:

INSERT INTO {table_name} (id) SELECT generate_series(1, {positive_rows});

Comment on lines 247 to 249
INSERT INTO {table_name} ("id")
SELECT "gs"
FROM generate_series({negative_base}, {negative_base} + {negative_rows} - 1) AS gs;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️

Prior to this change, if the table being repacked used negative pk values,
the backfill would cover the entire range from min(pk) to 0 (as well as the
positive values). When negative pk values have been used after positive
values are nearing exhaustion, it's likely that most of the negative range
is empty and therefore does not need to be backfilled.

This change checks for min(pk) and max(pk) separately for positive values
and negative values, to avoid backfilling empty negative ranges.
@timb07 timb07 force-pushed the backfill-positive-negative branch from ee0668b to 801b0e2 Compare June 18, 2025 00:55
@timb07 timb07 merged commit cdcd265 into main Jun 18, 2025
9 checks passed
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.

3 participants