Copilot commented on code in PR #2403:
URL: https://github.com/apache/age/pull/2403#discussion_r3114496362
##########
regress/sql/age_upgrade.sql:
##########
@@ -18,39 +18,49 @@
*/
--
--- Extension upgrade regression test
+-- Extension upgrade template regression test
--
--- This test validates the upgrade template (age--<VER>--y.y.y.sql) by:
--- 1. Dropping AGE and reinstalling at the synthetic "initial" version
--- (built from the version-bump commit's SQL — the "day-one" state)
--- 2. Creating three graphs with multiple labels, edges, GIN indexes,
--- and numeric properties that serve as integrity checksums
--- 3. Upgrading to the current (default) version via the stamped template
--- 4. Verifying all data, structure, and checksums survived the upgrade
+-- Validates the upgrade template (age--<VER>--y.y.y.sql) by comparing
+-- the pg_catalog entries of a fresh install against an upgraded install.
+-- If any function or relation exists in the fresh install but is missing
+-- after upgrade, the template is incomplete — it failed to create that
+-- object. This catches the case where a developer adds a new SQL function
+-- to sql/ and sql_files but forgets to add it to the upgrade template.
--
--- The Makefile builds:
--- age--<CURR>.sql from current HEAD's sql/sql_files (default)
--- age--<CURR>_initial.sql from the version-bump commit (synthetic)
--- age--<CURR>_initial--<CURR>.sql stamped from the upgrade template
---
--- All version discovery is dynamic — no hardcoded versions anywhere.
--- This test is version-agnostic and works on any branch for any version.
+-- The test compares functions (pg_proc) and relations (pg_class) in the
+-- ag_catalog schema. All comparison queries should return 0 rows.
--
LOAD 'age';
SET search_path TO ag_catalog;
--- Step 1: Clean up any state left by prior tests, then drop AGE entirely.
--- The --load-extension=age flag installed AGE at the current (default)
version.
--- We need to remove it so we can reinstall at the synthetic initial version.
+-- Step 1: Clean up any graphs left by prior tests.
SELECT drop_graph(name, true) FROM ag_graph ORDER BY name;
+
Review Comment:
The cleanup query will produce different, non-deterministic regression
output if any graphs exist (it returns 1 row per graph and may emit NOTICEs).
Since the expected output assumes 0 rows, this can fail the test for reasons
unrelated to upgrade validation. Consider performing the drops in a DO block
with PERFORM (and optionally suppressing NOTICEs) so the output is stable
regardless of prior graph state.
```suggestion
DO $$
DECLARE
graph_name ag_graph.name%TYPE;
BEGIN
PERFORM set_config('client_min_messages', 'warning', true);
FOR graph_name IN
SELECT name
FROM ag_graph
ORDER BY name
LOOP
PERFORM drop_graph(graph_name, true);
END LOOP;
END
$$;
```
##########
regress/sql/age_upgrade.sql:
##########
@@ -18,39 +18,49 @@
*/
--
--- Extension upgrade regression test
+-- Extension upgrade template regression test
--
--- This test validates the upgrade template (age--<VER>--y.y.y.sql) by:
--- 1. Dropping AGE and reinstalling at the synthetic "initial" version
--- (built from the version-bump commit's SQL — the "day-one" state)
--- 2. Creating three graphs with multiple labels, edges, GIN indexes,
--- and numeric properties that serve as integrity checksums
--- 3. Upgrading to the current (default) version via the stamped template
--- 4. Verifying all data, structure, and checksums survived the upgrade
+-- Validates the upgrade template (age--<VER>--y.y.y.sql) by comparing
+-- the pg_catalog entries of a fresh install against an upgraded install.
+-- If any function or relation exists in the fresh install but is missing
+-- after upgrade, the template is incomplete — it failed to create that
+-- object. This catches the case where a developer adds a new SQL function
+-- to sql/ and sql_files but forgets to add it to the upgrade template.
--
--- The Makefile builds:
--- age--<CURR>.sql from current HEAD's sql/sql_files (default)
--- age--<CURR>_initial.sql from the version-bump commit (synthetic)
--- age--<CURR>_initial--<CURR>.sql stamped from the upgrade template
---
--- All version discovery is dynamic — no hardcoded versions anywhere.
--- This test is version-agnostic and works on any branch for any version.
+-- The test compares functions (pg_proc) and relations (pg_class) in the
+-- ag_catalog schema. All comparison queries should return 0 rows.
--
LOAD 'age';
SET search_path TO ag_catalog;
--- Step 1: Clean up any state left by prior tests, then drop AGE entirely.
--- The --load-extension=age flag installed AGE at the current (default)
version.
--- We need to remove it so we can reinstall at the synthetic initial version.
+-- Step 1: Clean up any graphs left by prior tests.
SELECT drop_graph(name, true) FROM ag_graph ORDER BY name;
+
+-- Step 2: Snapshot the fresh install catalog — functions in ag_catalog.
+CREATE TEMP TABLE _fresh_funcs AS
+SELECT proname::text,
+ pg_get_function_identity_arguments(oid) AS args
+FROM pg_proc
+WHERE pronamespace = 'ag_catalog'::regnamespace
+ORDER BY proname, args;
+
+-- Step 3: Snapshot the fresh install catalog — relations in ag_catalog.
+CREATE TEMP TABLE _fresh_rels AS
+SELECT relname::text, relkind::text
+FROM pg_class
+WHERE relnamespace = 'ag_catalog'::regnamespace
+ AND relkind IN ('r', 'v', 'S')
Review Comment:
The pg_class snapshot is labeled as capturing “relations”, but the relkind
filter excludes indexes (relkind='i') and other relation kinds. ag_catalog
defines several indexes (e.g., in sql/age_main.sql), so an upgrade template
could miss creating an index/constraint and this test would still pass. Either
broaden the relkind filter to include the relation kinds you intend to validate
(at least indexes), or update the comments/docs to clearly state you’re only
comparing tables/views/sequences.
```suggestion
AND relkind IN ('r', 'v', 'S', 'i')
```
##########
regress/sql/age_upgrade.sql:
##########
@@ -219,88 +86,71 @@ BEGIN
IF curr_ver IS NULL THEN
RAISE EXCEPTION 'No default version found for upgrade test';
END IF;
-
EXECUTE format('ALTER EXTENSION age UPDATE TO %L', curr_ver);
END;
$$;
--- Step 7: Confirm version is now the default (current HEAD) version.
+-- Step 8: Confirm the upgrade succeeded.
SELECT installed_version = default_version AS upgraded_to_current
FROM pg_available_extensions WHERE name = 'age';
--- Step 8: Verify all data survived — reload and recheck.
-LOAD 'age';
-SET search_path TO ag_catalog, "$user", public;
-
--- Repeat integrity checksums — must match pre-upgrade values exactly.
-SELECT * FROM cypher('company', $$
- MATCH (n) WHERE n.val IS NOT NULL RETURN sum(n.val)
-$$) AS (company_val_sum_after agtype);
-
-SELECT * FROM cypher('network', $$
- MATCH (n:User) RETURN sum(n.score)
-$$) AS (network_score_sum_after agtype);
-
-SELECT * FROM cypher('routes', $$
- MATCH (c:City) RETURN sum(c.lat)
-$$) AS (routes_lat_sum_after agtype);
-
-SELECT sum(cnt)::int AS total_vertices_after FROM (
- SELECT count(*) AS cnt FROM cypher('company', $$ MATCH (n) RETURN n $$) AS
(n agtype)
- UNION ALL
- SELECT count(*) FROM cypher('network', $$ MATCH (n) RETURN n $$) AS (n
agtype)
- UNION ALL
- SELECT count(*) FROM cypher('routes', $$ MATCH (n) RETURN n $$) AS (n
agtype)
-) sub;
-
-SELECT sum(cnt)::int AS total_edges_after FROM (
- SELECT count(*) AS cnt FROM cypher('company', $$ MATCH ()-[e]->() RETURN e
$$) AS (e agtype)
- UNION ALL
- SELECT count(*) FROM cypher('network', $$ MATCH ()-[e]->() RETURN e $$) AS
(e agtype)
- UNION ALL
- SELECT count(*) FROM cypher('routes', $$ MATCH ()-[e]->() RETURN e $$) AS
(e agtype)
-) sub;
-
-SELECT count(*)::int AS total_labels_after
-FROM ag_label al JOIN ag_graph ag ON al.graph = ag.graphid
-WHERE ag.name <> '_ag_catalog';
-
--- Step 9: Verify specific structural queries across all three graphs.
-
--- company: management chain
-SELECT * FROM cypher('company', $$
- MATCH (boss:Employee)-[:MANAGES*]->(report:Employee)
- RETURN boss.name, report.name
- ORDER BY boss.name, report.name
-$$) AS (boss agtype, report agtype);
-
--- network: circular follow chain (proves full cycle survived)
-SELECT * FROM cypher('network', $$
- MATCH (a:User)-[:FOLLOWS]->(b:User)
- RETURN a.handle, b.handle
- ORDER BY a.handle
-$$) AS (follower agtype, followed agtype);
-
--- routes: all flights with distances (proves edge properties intact)
-SELECT * FROM cypher('routes', $$
- MATCH (a:Airport)-[f:FLIGHT]->(b:Airport)
- RETURN a.code, b.code, f.distance_km
- ORDER BY a.code, b.code
-$$) AS (origin agtype, dest agtype, dist agtype);
-
--- Step 10: Verify GIN indexes still exist after upgrade.
-SELECT indexname FROM pg_indexes
-WHERE schemaname IN ('company', 'network', 'routes')
- AND tablename IN ('Employee', 'User', 'Post', 'City')
- AND indexdef LIKE '%gin%'
-ORDER BY indexname;
-
--- Step 11: Cleanup and restore AGE at the default version for subsequent
tests.
-SELECT drop_graph('routes', true);
-SELECT drop_graph('network', true);
-SELECT drop_graph('company', true);
+-- Step 9: Snapshot the upgraded catalog — functions.
+CREATE TEMP TABLE _upgraded_funcs AS
+SELECT proname::text,
+ pg_get_function_identity_arguments(oid) AS args
+FROM pg_proc
+WHERE pronamespace = 'ag_catalog'::regnamespace
+ORDER BY proname, args;
+
+-- Step 10: Snapshot the upgraded catalog — relations.
+CREATE TEMP TABLE _upgraded_rels AS
+SELECT relname::text, relkind::text
+FROM pg_class
+WHERE relnamespace = 'ag_catalog'::regnamespace
+ AND relkind IN ('r', 'v', 'S')
+ORDER BY relname;
+
+-- Step 11: Functions in fresh install but MISSING after upgrade.
+-- Any rows here mean the upgrade template forgot to CREATE that function.
+SELECT f.proname || '(' || f.args || ')' AS missing_function
+FROM _fresh_funcs f
+LEFT JOIN _upgraded_funcs u USING (proname, args)
+WHERE u.proname IS NULL
+ORDER BY 1;
+
+-- Step 12: Functions in upgraded but NOT in fresh install.
+-- Any rows here mean the template creates something the fresh install doesn't.
+SELECT u.proname || '(' || u.args || ')' AS extra_function
+FROM _upgraded_funcs u
+LEFT JOIN _fresh_funcs f USING (proname, args)
+WHERE f.proname IS NULL
+ORDER BY 1;
+
+-- Step 13: Relations in fresh install but MISSING after upgrade.
+SELECT f.relname AS missing_relation
+FROM _fresh_rels f
+LEFT JOIN _upgraded_rels u USING (relname, relkind)
+WHERE u.relname IS NULL
+ORDER BY 1;
+
+-- Step 14: Relations in upgraded but NOT in fresh install.
+SELECT u.relname AS extra_relation
Review Comment:
The missing/extra relation diagnostics only print relname, but the
comparison key includes relkind. If a relation’s kind changes (e.g., table ↔
view), the output will show the same name in both lists without indicating why.
Consider including relkind in the SELECT output (or selecting both columns) to
make failures actionable.
```suggestion
SELECT f.relname || ' (' || f.relkind || ')' AS missing_relation
FROM _fresh_rels f
LEFT JOIN _upgraded_rels u USING (relname, relkind)
WHERE u.relname IS NULL
ORDER BY 1;
-- Step 14: Relations in upgraded but NOT in fresh install.
SELECT u.relname || ' (' || u.relkind || ')' AS extra_relation
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
