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

Issue #422: Make --error-on-invalid-index the default behavior #433

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

zubeyr94
Copy link
Contributor

Skipping repacking the invalid indexes could lead to corrupting the invalid indexes. In some rare cases, this might cause inserts to tables to fail as changes to a table continuously be applied to invalid indexes on it and updates to corrupted invalid indexes potentially might fail by the checks done during insertion. Therefore, by default do not repack a table with invalid indexes. Repack only user explicitly specifies --no-error-on-invalid-index.

bin/pg_repack.c Outdated
/* Enforce --error-on-invalid-index in all cases unless
* --no_error_on_invalid_index passed by itself
*/
if (error_on_invalid_index || !no_error_on_invalid_index) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR! What if we completely ignore error_on_invalid_index here and make it no-op as it was suggested previously #422 (comment), but keep it as a valid option and document it as a no-op in the documentation? WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs to stay as an option anyway. Can be marked as deprecated, and in 2-3 years can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think this makes sense. Released an updated version, which makes --error-on-invalid-index no-op and marked it as deprecated in helper and docs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! I'd merge the PR if no objections.

Skipping repacking the invalid indexes could lead to corrupting the
invalid indexes. In some rare cases, this might cause inserts to
tables to fail as changes to a table continuously be applied to
invalid indexes on it and updates to corrupted invalid indexes
potentially might fail by the checks done during insertion.
Therefore, by default do not repack a table with invalid indexes.
Repack only user explicitly specifies --no-error-on-invalid-index.
@zubeyr94 zubeyr94 force-pushed the skip-table-with-invalid-index branch from c81d575 to 55cfa77 Compare November 13, 2024 21:20
@za-arthur za-arthur merged commit 7e697c9 into reorg:master Nov 28, 2024
10 checks passed
@za-arthur
Copy link
Collaborator

@zubeyr94 I merged the PR. Thanks!

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