From 07e944d9d121b6b5ec2a8a2caa9c64a4e239257a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 19 Mar 2018 19:06:42 +0000 Subject: [PATCH 1/2] Use the Postgres way to get the version number Unsurprisingly, there was one. --- lib/pg_repack.sql.in | 24 ------------------------ regress/expected/repack.out | 2 +- regress/expected/repack_1.out | 2 +- regress/expected/repack_2.out | 2 +- regress/expected/repack_3.out | 2 +- regress/expected/repack_4.out | 2 +- regress/expected/repack_5.out | 2 +- regress/expected/repack_6.out | 2 +- regress/sql/repack.sql | 2 +- 9 files changed, 8 insertions(+), 32 deletions(-) diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 8a68b60..249547c 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -16,30 +16,6 @@ CREATE FUNCTION repack.version_sql() RETURNS text AS $$SELECT 'pg_repack REPACK_VERSION'::text$$ LANGUAGE SQL IMMUTABLE STRICT; -CREATE FUNCTION repack.pg_version(version_str text DEFAULT NULL) RETURNS integer -AS $$ - -- Return the server version number in a format similar to PG_VERSION_NUM. - -- Call with no argument for the server version, pass an argument for testing - select (case - when array_length(tokens, 1) = 2 then - to_char(tokens[1], 'FM00') || - '00' || to_char(tokens[2], 'FM00') - when array_length(tokens, 1) = 3 then - to_char(tokens[1], 'FM00') || - to_char(tokens[2], 'FM00') || - to_char(tokens[3], 'FM00') - else - -- This will raise an error which we can read - 'unexpected version string: ' || coalesce($1, version()) - end)::int - from ( - select string_to_array(substring( - split_part(coalesce($1, version()), ' ', 2) - from '\d+\.\d+(?:\.\d+)?'), '.')::int[] - ) as x (tokens); -$$ -LANGUAGE SQL IMMUTABLE; - CREATE AGGREGATE repack.array_accum ( sfunc = array_append, basetype = anyelement, diff --git a/regress/expected/repack.out b/regress/expected/repack.out index 3306ab0..a18c744 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename ------------ repack.out diff --git a/regress/expected/repack_1.out b/regress/expected/repack_1.out index df04ca8..87cc038 100644 --- a/regress/expected/repack_1.out +++ b/regress/expected/repack_1.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename -------------- repack_1.out diff --git a/regress/expected/repack_2.out b/regress/expected/repack_2.out index 0654079..3ceff4d 100644 --- a/regress/expected/repack_2.out +++ b/regress/expected/repack_2.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename -------------- repack_2.out diff --git a/regress/expected/repack_3.out b/regress/expected/repack_3.out index fea2c29..a7efdf8 100644 --- a/regress/expected/repack_3.out +++ b/regress/expected/repack_3.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename -------------- repack_3.out diff --git a/regress/expected/repack_4.out b/regress/expected/repack_4.out index 62a137e..6e868fa 100644 --- a/regress/expected/repack_4.out +++ b/regress/expected/repack_4.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename -------------- repack_4.out diff --git a/regress/expected/repack_5.out b/regress/expected/repack_5.out index d64771f..26d02e6 100644 --- a/regress/expected/repack_5.out +++ b/regress/expected/repack_5.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename -------------- repack_5.out diff --git a/regress/expected/repack_6.out b/regress/expected/repack_6.out index 3d57ffb..0da86a9 100644 --- a/regress/expected/repack_6.out +++ b/regress/expected/repack_6.out @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; filename -------------- repack_6.out diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index cdb35bd..cb744aa 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -12,7 +12,7 @@ select filename from (values (100000, 100003, 'repack_2.out'), (100003, 110000, 'repack_3.out') ) as x (min, max, filename) -where min <= repack.pg_version() and repack.pg_version() < max; +where current_setting('server_version_num')::int between min and max - 1; SET client_min_messages = warning; -- From c62d865c18e87d25b89067c6af0c156daf4bcc54 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 19 Mar 2018 19:39:37 +0000 Subject: [PATCH 2/2] Choose schema name visibility from running server version Guarantee the extension compiled on newer servers can be used on older ones. --- lib/repack.c | 55 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/lib/repack.c b/lib/repack.c index 036e310..0cb5f7c 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -35,6 +35,7 @@ #include "storage/lmgr.h" #include "utils/array.h" #include "utils/builtins.h" +#include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/relcache.h" @@ -353,31 +354,47 @@ get_relation_name(Oid relid) { Oid nsp = get_rel_namespace(relid); char *nspname; + char *strver; + int ver; + + /* Get the version of the running server (PG_VERSION_NUM would return + * the version we compiled the extension with) */ + strver = GetConfigOptionByName("server_version_num", NULL +#if PG_VERSION_NUM >= 90600 + , false /* missing_ok */ +#endif + ); + + ver = atoi(strver); + pfree(strver); /* * Relation names given by PostgreSQL core are always * qualified since some minor releases. Note that this change - * doesn't introduce to PostgreSQL 9.2 and 9.1 releases. + * wasn't introduced in PostgreSQL 9.2 and 9.1 releases. */ -#if ((PG_VERSION_NUM >= 100000 && PG_VERSION_NUM < 100003) || \ - (PG_VERSION_NUM >= 90600 && PG_VERSION_NUM < 90608) || \ - (PG_VERSION_NUM >= 90500 && PG_VERSION_NUM < 90512) || \ - (PG_VERSION_NUM >= 90400 && PG_VERSION_NUM < 90417) || \ - (PG_VERSION_NUM >= 90300 && PG_VERSION_NUM < 90322) || \ - (PG_VERSION_NUM >= 90200 && PG_VERSION_NUM < 90300) || \ - (PG_VERSION_NUM >= 90100 && PG_VERSION_NUM < 90200)) - /* Qualify the name if not visible in search path */ - if (RelationIsVisible(relid)) - nspname = NULL; + if ((ver >= 100000 && ver < 100003) || + (ver >= 90600 && ver < 90608) || + (ver >= 90500 && ver < 90512) || + (ver >= 90400 && ver < 90417) || + (ver >= 90300 && ver < 90322) || + (ver >= 90200 && ver < 90300) || + (ver >= 90100 && ver < 90200)) + { + /* Qualify the name if not visible in search path */ + if (RelationIsVisible(relid)) + nspname = NULL; + else + nspname = get_namespace_name(nsp); + } else - nspname = get_namespace_name(nsp); -#else - /* Qualify the name */ - if (OidIsValid(nsp)) - nspname = get_namespace_name(nsp); - else - nspname = NULL; -#endif + { + /* Always qualify the name */ + if (OidIsValid(nsp)) + nspname = get_namespace_name(nsp); + else + nspname = NULL; + } return quote_qualified_identifier(nspname, get_rel_name(relid)); }