From 625f6eb708faff1c1ca7872f6c3a4b5f10ccc138 Mon Sep 17 00:00:00 2001 From: Utkarash Singh Date: Mon, 5 Oct 2026 20:13:23 +0100 Subject: [PATCH 1/3] Force CREATE EXTENSION through the full upgrade chain Previously, a fresh CREATE EXTENSION installed straight from a standalone "latest version" script, while ALTER EXTENSION UPDATE on an existing install walked a separate chain of incremental scripts. These two paths were only kept in sync by hand, with nothing enforcing they actually produced the same end state. 0.1.1 and 0.1.2 only ever existed as standalone full scripts, with no update path connecting them - meaning a project already installed at 0.1.1 or 0.1.2 had no way to self-upgrade via ALTER EXTENSION UPDATE at all. This removes the standalone 0.1.1/0.1.2/0.2.0 scripts, keeping only 0.1.0 as the base install, with every later version reachable only via incremental upgrade scripts (adding the two that were missing: 0.1.1->0.1.2, 0.1.2->0.2.0). CREATE EXTENSION with no version, and ALTER EXTENSION UPDATE on an existing install, now both resolve through the same chain - so there's only one code path to keep correct, and it's exercised by every test that installs the extension at all, not just a dedicated upgrade test. 0.1.2->0.2.0 needs an explicit DROP FUNCTION first since the return signature changed (added the errors column) - confirmed via the actual Postgres error this produces without it ("cannot change return type of existing function"). Added two tests: - create_extension_default_version: unversioned CREATE EXTENSION resolves through the full chain to default_version. - upgrade_from_oldest_version: a real ALTER EXTENSION UPDATE from 0.1.0 walks the whole chain and the function works correctly afterward, not just a version-pinned fresh install. All 9 tests pass, including the 7 pre-existing ones unchanged. --- ...1.2.sql => index_advisor--0.1.1--0.1.2.sql | 0 index_advisor--0.1.1.sql | 159 ------------------ ...2.0.sql => index_advisor--0.1.2--0.2.0.sql | 2 + .../create_extension_default_version.out | 15 ++ test/expected/upgrade_from_oldest_version.out | 29 ++++ test/sql/create_extension_default_version.sql | 12 ++ test/sql/upgrade_from_oldest_version.sql | 24 +++ 7 files changed, 82 insertions(+), 159 deletions(-) rename index_advisor--0.1.2.sql => index_advisor--0.1.1--0.1.2.sql (100%) delete mode 100644 index_advisor--0.1.1.sql rename index_advisor--0.2.0.sql => index_advisor--0.1.2--0.2.0.sql (99%) create mode 100644 test/expected/create_extension_default_version.out create mode 100644 test/expected/upgrade_from_oldest_version.out create mode 100644 test/sql/create_extension_default_version.sql create mode 100644 test/sql/upgrade_from_oldest_version.sql diff --git a/index_advisor--0.1.2.sql b/index_advisor--0.1.1--0.1.2.sql similarity index 100% rename from index_advisor--0.1.2.sql rename to index_advisor--0.1.1--0.1.2.sql diff --git a/index_advisor--0.1.1.sql b/index_advisor--0.1.1.sql deleted file mode 100644 index aff8083..0000000 --- a/index_advisor--0.1.1.sql +++ /dev/null @@ -1,159 +0,0 @@ -create or replace function index_advisor( - query text -) - returns table ( - startup_cost_before jsonb, - startup_cost_after jsonb, - total_cost_before jsonb, - total_cost_after jsonb, - index_statements text[] - ) - volatile - language plpgsql - as $$ -declare - n_args int; - prepared_statement_name text = 'index_advisor_working_statement'; - hypopg_schema_name text = (select extnamespace::regnamespace::text from pg_extension where extname = 'hypopg'); - explain_plan_statement text; - rec record; - plan_initial jsonb; - plan_final jsonb; - statements text[] = '{}'; -begin - - -- Disallow multiple statements - if query ilike '%;%' then - raise exception 'query must not contain a semicolon'; - end if; - - -- Hack to support PostgREST because the prepared statement for args incorrectly defaults to text - query := replace(query, 'WITH pgrst_payload AS (SELECT $1 AS json_data)', 'WITH pgrst_payload AS (SELECT $1::json AS json_data)'); - - -- Create a prepared statement for the given query - deallocate all; - execute format('prepare %I as %s', prepared_statement_name, query); - - -- Detect how many arguments are present in the prepared statement - n_args = ( - select - coalesce(array_length(parameter_types, 1), 0) - from - pg_prepared_statements - where - name = prepared_statement_name - limit - 1 - ); - - -- Create a SQL statement that can be executed to collect the explain plan - explain_plan_statement = format( - 'set local plan_cache_mode = force_generic_plan; explain (format json) execute %I%s', - --'explain (format json) execute %I%s', - prepared_statement_name, - case - when n_args = 0 then '' - else format( - '(%s)', array_to_string(array_fill('null'::text, array[n_args]), ',') - ) - end - ); - - -- Store the query plan before any new indexes - execute explain_plan_statement into plan_initial; - - -- Create possible indexes - for rec in ( - with extension_regclass as ( - select - distinct objid as oid - from - pg_depend - where - deptype = 'e' - ) - select - pc.relnamespace::regnamespace::text as schema_name, - pc.relname as table_name, - pa.attname as column_name, - format( - 'select %I.hypopg_create_index($i$create index on %I.%I(%I)$i$)', - hypopg_schema_name, - pc.relnamespace::regnamespace::text, - pc.relname, - pa.attname - ) hypopg_statement - from - pg_catalog.pg_class pc - join pg_catalog.pg_attribute pa - on pc.oid = pa.attrelid - left join extension_regclass er - on pc.oid = er.oid - left join pg_index pi - on pc.oid = pi.indrelid - and (select array_agg(x) from unnest(pi.indkey) v(x)) = array[pa.attnum] - and pi.indexprs is null -- ignore expression indexes - and pi.indpred is null -- ignore partial indexes - where - pc.relnamespace::regnamespace::text not in ( -- ignore schema list - 'pg_catalog', 'pg_toast', 'information_schema' - ) - and er.oid is null -- ignore entities owned by extensions - and pc.relkind in ('r', 'm') -- regular tables, and materialized views - and pc.relpersistence = 'p' -- permanent tables (not unlogged or temporary) - and pa.attnum > 0 - and not pa.attisdropped - and pi.indrelid is null - and pa.atttypid in (20,16,1082,1184,1114,701,23,21,700,1083,2950,1700,25,18,1042,1043) - ) - loop - -- Create the hypothetical index - execute rec.hypopg_statement; - end loop; - - /* - for rec in select * from hypopg() - loop - raise notice '%', rec; - end loop; - */ - - -- Create a prepared statement for the given query - -- The original prepared statement MUST be dropped because its plan is cached - execute format('deallocate %I', prepared_statement_name); - execute format('prepare %I as %s', prepared_statement_name, query); - - -- Store the query plan after new indexes - execute explain_plan_statement into plan_final; - - --raise notice '%', plan_final; - - -- Idenfity referenced indexes in new plan - execute format( - 'select - coalesce(array_agg(hypopg_get_indexdef(indexrelid) order by indrelid, indkey::text), $i${}$i$::text[]) - from - %I.hypopg() - where - %s ilike ($i$%%$i$ || indexname || $i$%%$i$) - ', - hypopg_schema_name, - quote_literal(plan_final)::text - ) into statements; - - -- Reset all hypothetical indexes - perform hypopg_reset(); - - -- Reset prepared statements - deallocate all; - - return query values ( - (plan_initial -> 0 -> 'Plan' -> 'Startup Cost'), - (plan_final -> 0 -> 'Plan' -> 'Startup Cost'), - (plan_initial -> 0 -> 'Plan' -> 'Total Cost'), - (plan_final -> 0 -> 'Plan' -> 'Total Cost'), - statements::text[] - ); - -end; -$$; diff --git a/index_advisor--0.2.0.sql b/index_advisor--0.1.2--0.2.0.sql similarity index 99% rename from index_advisor--0.2.0.sql rename to index_advisor--0.1.2--0.2.0.sql index be45cfe..ca5cd49 100644 --- a/index_advisor--0.2.0.sql +++ b/index_advisor--0.1.2--0.2.0.sql @@ -1,3 +1,5 @@ +drop function index_advisor(text); + create or replace function index_advisor( query text ) diff --git a/test/expected/create_extension_default_version.out b/test/expected/create_extension_default_version.out new file mode 100644 index 0000000..6dd9e09 --- /dev/null +++ b/test/expected/create_extension_default_version.out @@ -0,0 +1,15 @@ +begin; + -- Unversioned CREATE EXTENSION must resolve through the full upgrade + -- chain (0.1.0 -> 0.1.1 -> 0.1.2 -> default_version), since only 0.1.0 + -- exists as a standalone install script - everything after that is only + -- reachable via incremental upgrade scripts. Confirms the chain is + -- unbroken end to end, not just reachable via an explicit version pin. + create extension index_advisor cascade; +NOTICE: installing required extension "hypopg" + select extversion from pg_extension where extname = 'index_advisor'; + extversion +------------ + 0.2.0 +(1 row) + +rollback; diff --git a/test/expected/upgrade_from_oldest_version.out b/test/expected/upgrade_from_oldest_version.out new file mode 100644 index 0000000..55345e1 --- /dev/null +++ b/test/expected/upgrade_from_oldest_version.out @@ -0,0 +1,29 @@ +begin; + -- Simulates a project that installed index_advisor at its very first + -- version and later runs ALTER EXTENSION ... UPDATE (e.g. during a + -- Postgres major-version upgrade, or a manual extension update) - + -- exercises the full 0.1.0 -> 0.1.1 -> 0.1.2 -> default_version upgrade + -- chain end to end, including the 0.1.2->default_version return-type + -- change (drop + recreate), not just a fresh install pinned at one + -- version. + create extension index_advisor version '0.1.0' cascade; +NOTICE: installing required extension "hypopg" + alter extension index_advisor update; + select extversion from pg_extension where extname = 'index_advisor'; + extversion +------------ + 0.2.0 +(1 row) + + create table public.book( + id int + ); + select index_advisor($$ + select * from book where id = $1 + $$); + index_advisor +--------------------------------------------------------------------------------- + (0.00,4.13,41.88,14.83,"{""CREATE INDEX ON public.book USING btree (id)""}",{}) +(1 row) + +rollback; diff --git a/test/sql/create_extension_default_version.sql b/test/sql/create_extension_default_version.sql new file mode 100644 index 0000000..bb5497a --- /dev/null +++ b/test/sql/create_extension_default_version.sql @@ -0,0 +1,12 @@ +begin; + + -- Unversioned CREATE EXTENSION must resolve through the full upgrade + -- chain (0.1.0 -> 0.1.1 -> 0.1.2 -> default_version), since only 0.1.0 + -- exists as a standalone install script - everything after that is only + -- reachable via incremental upgrade scripts. Confirms the chain is + -- unbroken end to end, not just reachable via an explicit version pin. + create extension index_advisor cascade; + + select extversion from pg_extension where extname = 'index_advisor'; + +rollback; diff --git a/test/sql/upgrade_from_oldest_version.sql b/test/sql/upgrade_from_oldest_version.sql new file mode 100644 index 0000000..0293c0d --- /dev/null +++ b/test/sql/upgrade_from_oldest_version.sql @@ -0,0 +1,24 @@ +begin; + + -- Simulates a project that installed index_advisor at its very first + -- version and later runs ALTER EXTENSION ... UPDATE (e.g. during a + -- Postgres major-version upgrade, or a manual extension update) - + -- exercises the full 0.1.0 -> 0.1.1 -> 0.1.2 -> default_version upgrade + -- chain end to end, including the 0.1.2->default_version return-type + -- change (drop + recreate), not just a fresh install pinned at one + -- version. + create extension index_advisor version '0.1.0' cascade; + + alter extension index_advisor update; + + select extversion from pg_extension where extname = 'index_advisor'; + + create table public.book( + id int + ); + + select index_advisor($$ + select * from book where id = $1 + $$); + +rollback; From 0bc39fe17c9bba3fb7d0596ae4f45461c4f0bcd3 Mon Sep 17 00:00:00 2001 From: Utkarash Singh Date: Mon, 5 Oct 2026 20:18:12 +0100 Subject: [PATCH 2/3] Fix CI and add PG17 to the test matrix actions/upload-artifact@v3 is deprecated and GitHub now hard-fails any run that uses it, which is why CI never actually executed on this repo (confirmed: zero runs ever, including on the historical merged PRs). Bumped it and actions/checkout to v4. Added PG17 to the matrix. This needed bumping the pinned hypopg version too - debian/1.3.1-2 doesn't compile against PG17 headers (tuplestore_donestoring no longer exists, a Datum/pointer type mismatch in hypopg_index.c). Verified 1.4.3 builds cleanly and passes the full suite on both PG15 and PG17. --- .github/workflows/test.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3627b8f..9ab46eb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,7 +12,7 @@ jobs: strategy: matrix: - pg: [15] + pg: [15, 17] steps: - name: Start PostgreSQL ${{ matrix.pg }} @@ -21,13 +21,13 @@ jobs: - name: Install build-dependencies run: apt-get install -y liblz4-dev libreadline-dev zlib1g-dev libzstd-dev - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: checkout hypopg - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: 'HypoPG/hypopg' - ref: 'debian/1.3.1-2' + ref: '1.4.3' path: ./hypopg - name: Create tests directory @@ -52,7 +52,7 @@ jobs: if: ${{ failure() }} run: cat regress/regression.diffs - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: always() with: name: regression.out From 2fe9f11e5ed34033ddb268dc459e6502e7e3a56b Mon Sep 17 00:00:00 2001 From: Utkarash Singh Date: Tue, 6 Oct 2026 11:42:24 +0100 Subject: [PATCH 3/3] Drop redundant 'or replace' after DROP FUNCTION Per Andrew's review comment - nothing exists to replace once the preceding drop function has run, so plain CREATE FUNCTION is clearer. --- index_advisor--0.1.2--0.2.0.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index_advisor--0.1.2--0.2.0.sql b/index_advisor--0.1.2--0.2.0.sql index ca5cd49..8acc08a 100644 --- a/index_advisor--0.1.2--0.2.0.sql +++ b/index_advisor--0.1.2--0.2.0.sql @@ -1,6 +1,6 @@ drop function index_advisor(text); -create or replace function index_advisor( +create function index_advisor( query text ) returns table (