Fix regex handling of tables with WITH storage parameters#65
Open
MathijsK93 wants to merge 66 commits into
Open
Fix regex handling of tables with WITH storage parameters#65MathijsK93 wants to merge 66 commits into
MathijsK93 wants to merge 66 commits into
Conversation
Split primary_keys and sequences cleanups
Add support for Rails 6.1
Move unique constraint to create table definitions
Keep pg_stat_statements extension for view
Add support for (MATERIALIZED) VIEWS indexes
…rted Fix/allow partitioned tables to be sorted
Add options to ignore some partitions in removal
Add CI test
… primary_key_test.rb
…r-constraints Add composite primary key support
Strip restrict
Handle new PG 18 not null constraints in pks
Fix named not null constraint on PK
Tables with WITH (...) storage parameters (e.g. autovacuum settings) end with `)` on one line followed by `WITH (...);\n` on the next. Two regexes didn't handle this terminator: - `indexes_after_tables`: end-of-table pattern only matched `);` or `PARTITION...);`, so indexes were extracted and deleted but never reinserted after the table. - `order_column_definitions`: lookahead only terminated on `);` or `)\nPARTITION...);`, causing the lazy column match to overshoot past the WITH clause into the next CREATE TABLE, stealing its columns. Both patterns now also match `)\nWITH [^\n]+;` as a valid table end. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…orage parameters The sub pattern \n\);\z only matched the normal table end `);`. With WITH storage parameters the table ends as `)` + newline + `WITH (...);\n`, so the `)` preceding `;\z` is inside the WITH clause, not the table-closing `)`. The sub never matched, causing constraints to be deleted from their ALTER TABLE location but never inserted inline. New pattern `(\n\)(?:;|\nWITH [^\n]+;))\z` anchors correctly on the table-closing `)` followed by either `;` or `\nWITH (...);\z`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
|
Hi @MathijsK93, thanks for the PR! Could you rebase on the upstream master branch so only your changes show up in the diff? |
Author
This PR was created by accident, but if you like I can push this fix upstream by creating a new PR? |
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
6 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.
Problem
Tables with
WITH (...)storage parameters (e.g. autovacuum settings) end theirCREATE TABLEblock as:```sql
)
WITH (autovacuum_vacuum_scale_factor='0.01', ...);
```
Two regexes in
CleanDumponly recognized);as the table terminator, breaking whenWITH (...)was present.Bug 1 —
indexes_after_tablesThe end-of-table pattern
(\);\n|PARTITION.+\);\n)didn't match)\nWITH (...);\n. The)line was consumed by the preceding(:?[^;\n]*\n)+group, leavingWITH (...);\nunmatched. Indexes were extracted and deleted from their original position but never reinserted after the table → all indexes for that table vanished from the dump.Bug 2 —
order_column_definitionsThe lookahead
(?=\n(\);|\)\nPARTITION.+\);)$)also didn't terminate at)\nWITH (...);\n. The lazy(?<columns>.+?)overshot past the storage parameters into the nextCREATE TABLEstatement, stealing its columns into the current table's column list.Fix
Both patterns now also accept
)\nWITH [^\n]+;as a valid table terminator.Test
Added
storage_parameters_test.rbthat verifies:WITH (...)🤖 Generated with Claude Code