Bump version to 6.0.0 and rename SQL scripts#401
Open
rasifr wants to merge 2 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR finalizes the Spock 6.0.0 release by updating the version constant from ChangesRelease 6.0.0 and Schema Upgrade Validation
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rasifr
force-pushed
the
task/SPOC-432/prepare-5.1.0-release
branch
from
868f54b to
1cf3438
Compare
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/tap/t/018_upgrade_schema_match.pl (1)
150-165: Minor: Temporary file cleanup on early exit.If
psql_querydies between creating$tmpfileand theunlink, the temp file is left behind. Consider using a block-scoped cleanup or wrapping in eval.♻️ Suggested improvement
sub psql_query { my ($port, $sql) = `@_`; my $tmpfile = "/tmp/spock_018_$$.sql"; + my $out; open my $fh, '>', $tmpfile or die "Cannot write $tmpfile: $!"; print $fh $sql; close $fh; - open $fh, '-|', "$PG_BIN/psql", '-X', '-t', '-A', - '-p', $port, '-d', $DB_NAME, '-U', $DB_USER, '-f', $tmpfile - or die "Cannot run psql: $!"; - my $out = join '', <$fh>; - close $fh; - unlink $tmpfile; + eval { + open $fh, '-|', "$PG_BIN/psql", '-X', '-t', '-A', + '-p', $port, '-d', $DB_NAME, '-U', $DB_USER, '-f', $tmpfile + or die "Cannot run psql: $!"; + $out = join '', <$fh>; + close $fh; + }; + my $err = $@; + unlink $tmpfile; + die $err if $err; chomp $out;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/tap/t/018_upgrade_schema_match.pl` around lines 150 - 165, psql_query currently writes a temp file and may leave it behind if the function dies before the final unlink; modify psql_query to guarantee cleanup by using a safe temporary-file helper (e.g., File::Temp->new or a block-scoped temp file) or wrap the write/psql invocation in an eval/finally-like construct (local $@; eval { ... }; my $err = $@; unlink $tmpfile if -e $tmpfile; die $err if $err) so that the temp file is always removed; reference the psql_query function and the $tmpfile variable when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/tap/t/018_upgrade_schema_match.pl`:
- Around line 150-165: psql_query currently writes a temp file and may leave it
behind if the function dies before the final unlink; modify psql_query to
guarantee cleanup by using a safe temporary-file helper (e.g., File::Temp->new
or a block-scoped temp file) or wrap the write/psql invocation in an
eval/finally-like construct (local $@; eval { ... }; my $err = $@; unlink
$tmpfile if -e $tmpfile; die $err if $err) so that the temp file is always
removed; reference the psql_query function and the $tmpfile variable when making
the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87b0828a-fe94-4595-8692-810148ddddff
⛔ Files ignored due to path filters (2)
tests/regress/expected/init_1.outis excluded by!**/*.outtests/regress/expected/init_fail.outis excluded by!**/*.out
📒 Files selected for processing (6)
include/spock.hsql/spock--5.0.6--5.1.0.sqlsql/spock--5.1.0.sqltests/regress/sql/init_fail.sqltests/tap/scheduletests/tap/t/018_upgrade_schema_match.pl
mason-sharp
changed the title
Member
|
We will also need to pull in 5.0.7 |
rasifr
force-pushed
the
task/SPOC-432/prepare-5.1.0-release
branch
from
1cf3438 to
e49aa36
Compare
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sql/spock--5.0.6--5.0.7.sql`:
- Line 37: Replace the invalid PL/pgSQL assignment-from-query "target_id :=
node_id FROM spock.node_info();" with a proper SELECT ... INTO form; e.g. use
"SELECT node_id INTO target_id FROM spock.node_info();" (or "SELECT INTO STRICT"
if you expect exactly one row) for both occurrences referencing target_id,
node_id and spock.node_info() (the instances around lines 37 and 109).
In `@tests/tap/t/018_upgrade_schema_match.pl`:
- Around line 276-280: The stop_pg helper ignores failures from pg_ctl which can
leave the old postmaster running; modify stop_pg (the function `stop_pg`) to
check the result/exit status of `run_logged("$PG_BIN/pg_ctl", 'stop', ...)` and
fail the test (or die) if pg_ctl returns non-zero, and optionally wait/retry
until the postmaster is confirmed stopped (e.g. poll for PID file removal or use
`pg_ctl status`) before returning so subsequent phase 6 upgrade assertions won't
run against a still-running old postmaster.
- Around line 156-161: The current code reads psql output from a piped
filehandle ($fh) but never checks the exit status of the spawned psql process,
which can return non-zero and produce empty output; after close $fh add a check
of the child's exit status (e.g., inspect $? and die with a clear error
including $tmpfile, the command and exit code) so that when the psql invocation
(the open using "$PG_BIN/psql" with $tmpfile) fails the test fails fast instead
of using empty output for schema matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99ba0d47-b3ac-43ef-b218-8c30016e8fcb
⛔ Files ignored due to path filters (2)
tests/regress/expected/init_1.outis excluded by!**/*.outtests/regress/expected/init_fail.outis excluded by!**/*.out
📒 Files selected for processing (7)
include/spock.hsql/spock--5.0.6--5.0.7.sqlsql/spock--5.0.7--6.0.0.sqlsql/spock--6.0.0.sqltests/regress/sql/init_fail.sqltests/tap/scheduletests/tap/t/018_upgrade_schema_match.pl
✅ Files skipped from review due to trivial changes (2)
- include/spock.h
- tests/tap/schedule
| IF origin_id IS NULL THEN | ||
| RAISE EXCEPTION 'Invalid NULL origin_id'; | ||
| END IF; | ||
| target_id := node_id FROM spock.node_info(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expect no matches after the fix.
rg -nP ':\=\s*[A-Za-z_][A-Za-z0-9_]*\s+FROM\s+' sql/spock--5.0.6--5.0.7.sqlRepository: pgEdge/spock
Length of output: 173
🏁 Script executed:
cat -n sql/spock--5.0.6--5.0.7.sql | head -50Repository: pgEdge/spock
Length of output: 2380
🏁 Script executed:
cat -n sql/spock--5.0.6--5.0.7.sql | sed -n '25,50p'Repository: pgEdge/spock
Length of output: 1133
🏁 Script executed:
cat -n sql/spock--5.0.6--5.0.7.sql | sed -n '100,120p'Repository: pgEdge/spock
Length of output: 937
Fix invalid PL/pgSQL assignment-from-query syntax at lines 37 and 109.
The := ... FROM ... construct is not valid PL/pgSQL and will fail when the procedures are created during the upgrade.
Proposed fix
- target_id := node_id FROM spock.node_info();
+ SELECT node_id INTO target_id FROM spock.node_info();- origin_id := node_id FROM spock.node WHERE node_name = origin;
+ SELECT node_id INTO origin_id FROM spock.node WHERE node_name = origin;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| target_id := node_id FROM spock.node_info(); | |
| SELECT node_id INTO target_id FROM spock.node_info(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sql/spock--5.0.6--5.0.7.sql` at line 37, Replace the invalid PL/pgSQL
assignment-from-query "target_id := node_id FROM spock.node_info();" with a
proper SELECT ... INTO form; e.g. use "SELECT node_id INTO target_id FROM
spock.node_info();" (or "SELECT INTO STRICT" if you expect exactly one row) for
both occurrences referencing target_id, node_id and spock.node_info() (the
instances around lines 37 and 109).
Comment on lines
+156
to
+161
| open $fh, '-|', "$PG_BIN/psql", '-X', '-t', '-A', | ||
| '-p', $port, '-d', $DB_NAME, '-U', $DB_USER, '-f', $tmpfile | ||
| or die "Cannot run psql: $!"; | ||
| my $out = join '', <$fh>; | ||
| close $fh; | ||
| unlink $tmpfile; |
There was a problem hiding this comment.
Fail fast when psql_query execution fails.
Line 156–Line 161 do not validate the piped psql exit status. A failed query can collapse to empty output and produce false-positive schema matches.
💡 Proposed fix
my $out = join '', <$fh>;
- close $fh;
+ my $ok = close $fh;
+ my $rc = $? >> 8;
unlink $tmpfile;
+ die "psql query failed (rc=$rc) for $tmpfile" if !$ok || $rc != 0;
chomp $out;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/tap/t/018_upgrade_schema_match.pl` around lines 156 - 161, The current
code reads psql output from a piped filehandle ($fh) but never checks the exit
status of the spawned psql process, which can return non-zero and produce empty
output; after close $fh add a check of the child's exit status (e.g., inspect $?
and die with a clear error including $tmpfile, the command and exit code) so
that when the psql invocation (the open using "$PG_BIN/psql" with $tmpfile)
fails the test fails fast instead of using empty output for schema matching.
Comment on lines
+276
to
+280
| sub stop_pg { | ||
| my ($datadir) = @_; | ||
| run_logged("$PG_BIN/pg_ctl", 'stop', '-D', $datadir, '-m', 'fast', '-w'); | ||
| sleep 2; | ||
| } |
There was a problem hiding this comment.
Check pg_ctl stop result before continuing upgrade flow.
Line 276–Line 280 ignore stop failures; Phase 6 may continue with an old postmaster still running, making the binary-swap/version assertions flaky.
💡 Proposed fix
sub stop_pg {
my ($datadir) = `@_`;
- run_logged("$PG_BIN/pg_ctl", 'stop', '-D', $datadir, '-m', 'fast', '-w');
+ my $rc = run_logged("$PG_BIN/pg_ctl", 'stop', '-D', $datadir, '-m', 'fast', '-w');
+ BAIL_OUT("Failed to stop postgres datadir=$datadir (rc=$rc). See $LOG_FILE") if $rc;
sleep 2;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| sub stop_pg { | |
| my ($datadir) = @_; | |
| run_logged("$PG_BIN/pg_ctl", 'stop', '-D', $datadir, '-m', 'fast', '-w'); | |
| sleep 2; | |
| } | |
| sub stop_pg { | |
| my ($datadir) = `@_`; | |
| my $rc = run_logged("$PG_BIN/pg_ctl", 'stop', '-D', $datadir, '-m', 'fast', '-w'); | |
| BAIL_OUT("Failed to stop postgres datadir=$datadir (rc=$rc). See $LOG_FILE") if $rc; | |
| sleep 2; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/tap/t/018_upgrade_schema_match.pl` around lines 276 - 280, The stop_pg
helper ignores failures from pg_ctl which can leave the old postmaster running;
modify stop_pg (the function `stop_pg`) to check the result/exit status of
`run_logged("$PG_BIN/pg_ctl", 'stop', ...)` and fail the test (or die) if pg_ctl
returns non-zero, and optionally wait/retry until the postmaster is confirmed
stopped (e.g. poll for PID file removal or use `pg_ctl status`) before returning
so subsequent phase 6 upgrade assertions won't run against a still-running old
postmaster.
- Rename sql/spock--6.0.0-devel.sql → spock--6.0.0.sql - Rename sql/spock--5.0.6--6.0.0-devel.sql → spock--5.0.8--6.0.0.sql - Add sql/spock--5.0.6--5.0.7.sql step upgrade script - Add sql/spock--5.0.7--5.0.8.sql step upgrade script (no schema changes in 5.0.8) - Bump version in include/spock.h - Update regression test fixtures for 6.0.0 version strings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ma parity TAP test that installs spock 5.0.8, upgrades to 6.0.0, and compares the resulting schema against a fresh 6.0.0 install — covering tables, columns, functions, views, sequences, view definitions, and function bodies. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rasifr
force-pushed
the
task/SPOC-432/prepare-5.1.0-release
branch
from
e49aa36 to
09929f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sql/spock--5.0.8--6.0.0.sql (1)
289-323:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace
%Iwith%sfor theregclassparameterrel.Since
relis declared asregclass(line 220), it renders as a schema-qualified relation name. Using%Itreats the entire text as a single identifier, producing"public.my_table"instead ofpublic.my_table, which breaks both theSECURITY LABEL ON COLUMNandALTER TABLEstatements. Use%sinstead, as regclass output is already safely-escaped.Fix locations
- Line 290:
SECURITY LABEL FOR spock ON COLUMN %I.%I→ change first%Ito%s- Line 293:
SECURITY LABEL FOR spock ON COLUMN %I.%I→ change first%Ito%s- Line 316:
ALTER TABLE %I REPLICA IDENTITY FULL→ change%Ito%s- Line 323:
ALTER TABLE %I REPLICA IDENTITY DEFAULT→ change%Ito%s🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sql/spock--5.0.8--6.0.0.sql` around lines 289 - 323, The SQL uses the regclass variable rel with format() and %I, which quotes the whole schema-qualified name; update the format calls that build sqlstring and ALTER TABLE to use %s for rel instead of %I: change the first %I to %s in both SECURITY LABEL FOR spock ON COLUMN format() calls (the ones that build sqlstring) and change %I to %s in the two ALTER TABLE format() calls so the regclass value renders as an unquoted schema-qualified name; keep %I for att_name and other true identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sql/spock--5.0.8--6.0.0.sql`:
- Around line 1-4: The script header and user message still target 5.0.8->6.0.0;
change both to reflect the correct upgrade source 5.0.7->6.0.0 by renaming the
file comment string "/* spock--5.0.8--6.0.0.sql */" to "/*
spock--5.0.7--6.0.0.sql */" (and if the actual filename is the old form, rename
the file accordingly) and update the psql echo text that contains "ALTER
EXTENSION spock UPDATE TO '6.0.0'" to clearly reference the 5.0.7 source (e.g.,
mirror the filename change in the message so the script and its user-facing
guidance consistently indicate 5.0.7 -> 6.0.0).
In `@tests/tap/t/018_upgrade_schema_match.pl`:
- Around line 286-330: The test's catalog queries miss index definitions and
sequence properties; update the diff queries by adding a new Q_INDEXES (e.g.,
query pg_catalog.pg_indexes or join pg_class/pg_index/pg_attribute) that returns
a deterministic string per index including schemaname.table, indexname, indexdef
(or columns/order, uniqueness, predicate, expression) ordered by index name, and
extend Q_SEQUENCES to include sequence attributes (data_type/udt_name,
start_value, minimum_value, maximum_value, increment_by, is_cycled,
last_value/owned_by if relevant) so the test will detect changes like
spock.resolutions(log_time) index removal or differences in
spock.sub_id_generator AS integer/CYCLE/start; apply the same sequence expansion
where the other query copy exists (lines noted in the comment).
---
Outside diff comments:
In `@sql/spock--5.0.8--6.0.0.sql`:
- Around line 289-323: The SQL uses the regclass variable rel with format() and
%I, which quotes the whole schema-qualified name; update the format calls that
build sqlstring and ALTER TABLE to use %s for rel instead of %I: change the
first %I to %s in both SECURITY LABEL FOR spock ON COLUMN format() calls (the
ones that build sqlstring) and change %I to %s in the two ALTER TABLE format()
calls so the regclass value renders as an unquoted schema-qualified name; keep
%I for att_name and other true identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 823492b3-11ae-4fbc-842a-10d682c2c343
⛔ Files ignored due to path filters (2)
tests/regress/expected/init_1.outis excluded by!**/*.outtests/regress/expected/init_fail.outis excluded by!**/*.out
📒 Files selected for processing (8)
include/spock.hsql/spock--5.0.6--5.0.7.sqlsql/spock--5.0.7--5.0.8.sqlsql/spock--5.0.8--6.0.0.sqlsql/spock--6.0.0.sqltests/regress/sql/init_fail.sqltests/tap/scheduletests/tap/t/018_upgrade_schema_match.pl
✅ Files skipped from review due to trivial changes (4)
- sql/spock--5.0.7--5.0.8.sql
- tests/tap/schedule
- include/spock.h
- tests/regress/sql/init_fail.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- sql/spock--5.0.6--5.0.7.sql
Comment on lines
+1
to
+4
| /* spock--5.0.8--6.0.0.sql */ | ||
|
|
||
| -- complain if script is sourced in psql, rather than via ALTER EXTENSION | ||
| \echo Use "ALTER EXTENSION spock UPDATE TO '6.0.0-devel'" to load this file. \quit | ||
| \echo Use "ALTER EXTENSION spock UPDATE TO '6.0.0'" to load this file. \quit |
There was a problem hiding this comment.
Retarget this direct upgrade to 5.0.7 -> 6.0.0.
This script still advertises 5.0.8 -> 6.0.0, but the release chain in this PR is 5.0.6 -> 5.0.7 -> 6.0.0. Leaving it here means the shipped upgrade artifact and the new validation path are aimed at the wrong source version.
Based on learnings: Treat intermediate upgrade SQL scripts as part of the upgrade chain and only flag issues as bugs if they affect the final upgrade outcome or are not corrected later in the chain.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sql/spock--5.0.8--6.0.0.sql` around lines 1 - 4, The script header and user
message still target 5.0.8->6.0.0; change both to reflect the correct upgrade
source 5.0.7->6.0.0 by renaming the file comment string "/*
spock--5.0.8--6.0.0.sql */" to "/* spock--5.0.7--6.0.0.sql */" (and if the
actual filename is the old form, rename the file accordingly) and update the
psql echo text that contains "ALTER EXTENSION spock UPDATE TO '6.0.0'" to
clearly reference the 5.0.7 source (e.g., mirror the filename change in the
message so the script and its user-facing guidance consistently indicate 5.0.7
-> 6.0.0).
Comment on lines
+286
to
+330
| my $Q_TABLES = <<'SQL'; | ||
| SELECT string_agg(table_name, E'\n' ORDER BY table_name) | ||
| FROM information_schema.tables | ||
| WHERE table_schema = 'spock' AND table_type = 'BASE TABLE' | ||
| SQL | ||
|
|
||
| # udt_name catches text vs text[] mismatches (_text = text[]) | ||
| my $Q_COLUMNS = <<'SQL'; | ||
| SELECT string_agg( | ||
| table_name || '.' || column_name || ' ' || udt_name | ||
| || CASE WHEN is_nullable = 'NO' THEN ' NOT NULL' ELSE '' END, | ||
| E'\n' ORDER BY table_name, ordinal_position) | ||
| FROM information_schema.columns | ||
| WHERE table_schema = 'spock' | ||
| SQL | ||
|
|
||
| my $Q_FUNCTIONS = <<'SQL'; | ||
| SELECT string_agg( | ||
| proname | ||
| || '(' || pg_catalog.pg_get_function_arguments(oid) || ')' | ||
| || ':' || prokind::text, | ||
| E'\n' ORDER BY proname, pg_catalog.pg_get_function_arguments(oid)) | ||
| FROM pg_catalog.pg_proc | ||
| WHERE pronamespace = 'spock'::regnamespace | ||
| SQL | ||
|
|
||
| my $Q_VIEWS = <<'SQL'; | ||
| SELECT string_agg(viewname, E'\n' ORDER BY viewname) | ||
| FROM pg_catalog.pg_views | ||
| WHERE schemaname = 'spock' | ||
| SQL | ||
|
|
||
| my $Q_SEQUENCES = <<'SQL'; | ||
| SELECT string_agg(sequence_name, E'\n' ORDER BY sequence_name) | ||
| FROM information_schema.sequences | ||
| WHERE sequence_schema = 'spock' | ||
| SQL | ||
|
|
||
| # View definitions (catches changes to view SQL) | ||
| my $Q_VIEW_DEFS = <<'SQL'; | ||
| SELECT string_agg(viewname || ':' || regexp_replace(definition, '\s+', ' ', 'g'), | ||
| E'\n' ORDER BY viewname) | ||
| FROM pg_catalog.pg_views | ||
| WHERE schemaname = 'spock' | ||
| SQL |
There was a problem hiding this comment.
The catalog diff still misses index definitions and sequence properties.
Right now Q_SEQUENCES only compares names, and there is no index comparison at all. This test can still pass if the upgraded schema is missing spock.resolutions(log_time) or if spock.sub_id_generator differs in AS integer / CYCLE / start settings.
💡 Suggested expansion
+my $Q_INDEXES = <<'SQL';
+SELECT string_agg(
+ indexname || ':' || regexp_replace(indexdef, '\s+', ' ', 'g'),
+ E'\n' ORDER BY indexname)
+FROM pg_catalog.pg_indexes
+WHERE schemaname = 'spock'
+SQL
+
my $Q_SEQUENCES = <<'SQL';
SELECT string_agg(sequence_name, E'\n' ORDER BY sequence_name)
FROM information_schema.sequences
WHERE sequence_schema = 'spock'
SQL
+
+my $Q_SEQUENCE_DEFS = <<'SQL';
+SELECT string_agg(
+ sequencename || ':' || data_type || ':' || start_value || ':' ||
+ minimum_value || ':' || maximum_value || ':' || increment_by || ':' || cycle,
+ E'\n' ORDER BY sequencename)
+FROM pg_catalog.pg_sequences
+WHERE schemaname = 'spock'
+SQL
@@
compare_category('Sequences',
psql_query($PORT_UPG, $Q_SEQUENCES),
psql_query($PORT_NEW, $Q_SEQUENCES));
+
+compare_category('Sequence definitions',
+ psql_query($PORT_UPG, $Q_SEQUENCE_DEFS),
+ psql_query($PORT_NEW, $Q_SEQUENCE_DEFS));
+
+compare_category('Indexes',
+ psql_query($PORT_UPG, $Q_INDEXES),
+ psql_query($PORT_NEW, $Q_INDEXES));Also applies to: 445-451
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/tap/t/018_upgrade_schema_match.pl` around lines 286 - 330, The test's
catalog queries miss index definitions and sequence properties; update the diff
queries by adding a new Q_INDEXES (e.g., query pg_catalog.pg_indexes or join
pg_class/pg_index/pg_attribute) that returns a deterministic string per index
including schemaname.table, indexname, indexdef (or columns/order, uniqueness,
predicate, expression) ordered by index name, and extend Q_SEQUENCES to include
sequence attributes (data_type/udt_name, start_value, minimum_value,
maximum_value, increment_by, is_cycled, last_value/owned_by if relevant) so the
test will detect changes like spock.resolutions(log_time) index removal or
differences in spock.sub_id_generator AS integer/CYCLE/start; apply the same
sequence expansion where the other query copy exists (lines noted in the
comment).
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.
Bump version to 6.0.0 and add upgrade path from 5.0.7
SPOCK_VERSIONto"6.0.0"andSPOCK_VERSION_NUMto60000ininclude/spock.hspock--6.0.0-devel.sql→spock--6.0.0.sqlandspock--5.0.6--6.0.0-devel.sql→spock--5.0.7--6.0.0.sqlspock--5.0.6--5.0.7.sqlstep upgrade script (pause/resume workers,wait_for_sync_eventrewrite,sync_eventoptionaltransactionalarg,sub_skip_schemacolumn relabel)progress/lag_trackerviews, addssub_id_generatorsequence, migrates conflict type names, and addscleanup_resolutions+ subscription stats functions018_upgrade_schema_matchto verify that a 5.0.7 → 6.0.0 upgrade produces a schema identical to a fresh 6.0.0 install