public inbox for [email protected]help / color / mirror / Atom feed
Re: [PATCH] ltree hash functions 13+ messages / 7 participants [nested] [flat]
* Re: [PATCH] ltree hash functions @ 2023-06-17 18:19 Tom Lane <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Tom Lane @ 2023-06-17 18:19 UTC (permalink / raw) To: Tomas Vondra <[email protected]>; +Cc: Tommy Pavlicek <[email protected]>; [email protected] Tomas Vondra <[email protected]> writes: > I guess the "correct" solution would be to extend ALTER OPERATOR. I > wonder why it's not supported - it's clearly an intentional decision > (per comment in AlterOperator). So what might break if this changes for > an existing operator? This code was added by commit 321eed5f0. The thread leading up to that commit is here: https://www.postgresql.org/message-id/flat/3348985.V7xMLFDaJO%40dinodell There are some nontrivial concerns in there about breaking the semantics of existing exclusion constraints, for instance. I think we mostly rejected the concern about invalidation of cached plans as already-covered, but that wasn't the only problem. However, I think we could largely ignore the issues if we restricted ALTER OPERATOR to only add commutator, negator, hashes, or merges properties to operators that lacked them before --- which'd be the primary if not only use-case anyway. That direction can't break anything. regards, tom lane ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-06-17 19:57 Tomas Vondra <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Tomas Vondra @ 2023-06-17 19:57 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Tommy Pavlicek <[email protected]>; [email protected] On 6/17/23 20:19, Tom Lane wrote: > Tomas Vondra <[email protected]> writes: >> I guess the "correct" solution would be to extend ALTER OPERATOR. I >> wonder why it's not supported - it's clearly an intentional decision >> (per comment in AlterOperator). So what might break if this changes for >> an existing operator? > > This code was added by commit 321eed5f0. The thread leading up to > that commit is here: > > https://www.postgresql.org/message-id/flat/3348985.V7xMLFDaJO%40dinodell > > There are some nontrivial concerns in there about breaking the > semantics of existing exclusion constraints, for instance. I think > we mostly rejected the concern about invalidation of cached plans > as already-covered, but that wasn't the only problem. > > However, I think we could largely ignore the issues if we restricted > ALTER OPERATOR to only add commutator, negator, hashes, or merges > properties to operators that lacked them before --- which'd be the > primary if not only use-case anyway. That direction can't break > anything. > Sound reasonable. Tommy, are you interested in extending ALTER OPERATOR to allow this, which would also allow fixing the ltree operator? regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-06-19 09:18 Tommy Pavlicek <[email protected]> parent: Tomas Vondra <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Tommy Pavlicek @ 2023-06-19 09:18 UTC (permalink / raw) To: Tomas Vondra <[email protected]>; +Cc: Tom Lane <[email protected]>; [email protected] > > FWIW the CREATE OPERATOR documentation only talks about hash joins for HASHES, maybe it should be updated to also mention hash aggregates? I think I might have been a bit unclear here, the hash aggregate does work without altering the operator so it's just the join that's blocked. Sorry about the confusion. I wonder what's the use case for this. I wonder how often people join on > ltree, for example. Did you just notice ltree can't hash and decided to > fix that, or do you have a practical use case / need for this feature? I mostly want to add hash indexes. Beyond selecting specific values, you can use them to get ancestors (trim the path and do an exact select) and descendents (using a functional index calculating the parent path for each row). For example, I've found it can be faster to calculate the path of every ancestor and use select ltree path = ANY([ancestor paths]) compared to using a gist index. It's not ideal, but unfortunately I've found that with enough rows, gist indexes get very large and slow. Btree indexes are better, but for ltree they can still be up to around 10x bigger than a hash index. I've also seen ltree hash indexes outperform btree indexes in very large tables, but I suspect in most cases they'll be similar. Tommy, are you interested in extending ALTER OPERATOR to allow this, > which would also allow fixing the ltree operator? Yes, I can do that. I took a look over the code and email thread and it seems like it should be relatively straight forward. I'll put a patch together for that and then update this patch to alter the operator. On Sat, Jun 17, 2023 at 9:57 PM Tomas Vondra <[email protected]> wrote: > > > On 6/17/23 20:19, Tom Lane wrote: > > Tomas Vondra <[email protected]> writes: > >> I guess the "correct" solution would be to extend ALTER OPERATOR. I > >> wonder why it's not supported - it's clearly an intentional decision > >> (per comment in AlterOperator). So what might break if this changes for > >> an existing operator? > > > > This code was added by commit 321eed5f0. The thread leading up to > > that commit is here: > > > > https://www.postgresql.org/message-id/flat/3348985.V7xMLFDaJO%40dinodell > > > > There are some nontrivial concerns in there about breaking the > > semantics of existing exclusion constraints, for instance. I think > > we mostly rejected the concern about invalidation of cached plans > > as already-covered, but that wasn't the only problem. > > > > However, I think we could largely ignore the issues if we restricted > > ALTER OPERATOR to only add commutator, negator, hashes, or merges > > properties to operators that lacked them before --- which'd be the > > primary if not only use-case anyway. That direction can't break > > anything. > > > > Sound reasonable. > > Tommy, are you interested in extending ALTER OPERATOR to allow this, > which would also allow fixing the ltree operator? > > > regards > > -- > Tomas Vondra > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-07-06 08:18 Daniel Gustafsson <[email protected]> parent: Tommy Pavlicek <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Daniel Gustafsson @ 2023-07-06 08:18 UTC (permalink / raw) To: Tommy Pavlicek <[email protected]>; +Cc: Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] > On 19 Jun 2023, at 11:18, Tommy Pavlicek <[email protected]> wrote: > Tommy, are you interested in extending ALTER OPERATOR to allow this, > which would also allow fixing the ltree operator? > > Yes, I can do that. I took a look over the code and email thread and it seems like it should be relatively straight forward. I'll put a patch together for that and then update this patch to alter the operator. Did you have a chance to look at this for an updated patch for this commitfest? -- Daniel Gustafsson ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-11-28 22:08 Tommy Pavlicek <[email protected]> parent: Daniel Gustafsson <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Tommy Pavlicek @ 2023-11-28 22:08 UTC (permalink / raw) To: [email protected]; +Cc: Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] On Thu, Jul 6, 2023 at 2:18 AM Daniel Gustafsson <[email protected]> wrote: > > > On 19 Jun 2023, at 11:18, Tommy Pavlicek <[email protected]> wrote: > > > Tommy, are you interested in extending ALTER OPERATOR to allow this, > > which would also allow fixing the ltree operator? > > > > Yes, I can do that. I took a look over the code and email thread and it seems like it should be relatively straight forward. I'll put a patch together for that and then update this patch to alter the operator. > > Did you have a chance to look at this for an updated patch for this commitfest? I finally had a chance to look at this and I've updated the patch to alter the = operator to enable hash joins. This is ready to be looked at now. Is there anything I need to do to move this forward? Cheers, Tommy Attachments: [application/octet-stream] 0001-ltree-hash-v2.patch (9.1K, ../../CAEhP-W-dqWKLNUQEGSb+06mo-5hP6MTCBeL4RkxosVh+8pQQfg@mail.gmail.com/2-0001-ltree-hash-v2.patch) download | inline diff: diff --git a/contrib/ltree/Makefile b/contrib/ltree/Makefile index 770769a730..f50f2c9470 100644 --- a/contrib/ltree/Makefile +++ b/contrib/ltree/Makefile @@ -14,7 +14,7 @@ OBJS = \ ltxtquery_op.o EXTENSION = ltree -DATA = ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql +DATA = ltree--1.2--1.3.sql ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql PGFILEDESC = "ltree - hierarchical label data type" HEADERS = ltree.h diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index 984cd030cf..b0f99afea1 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1433,8 +1433,27 @@ SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; g.b.c.d.e (1 row) +-- Check that the ltree_hash() and ltree_hash_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, ltree_hash(v)::bit(32) as standard, + ltree_hash_extended(v, 0)::bit(32) as extended0, + ltree_hash_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE ltree_hash(v)::bit(32) != ltree_hash_extended(v, 0)::bit(32) + OR ltree_hash(v)::bit(32) = ltree_hash_extended(v, 1)::bit(32); + value | standard | extended0 | extended1 +-------+----------+-----------+----------- +(0 rows) + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + count +------- + 1006 +(1 row) + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; t ---------------------------------- @@ -7832,6 +7851,26 @@ SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; 23.3.32.21.5.14.10.17.1 (4 rows) +drop index tstidx; +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + t +------ + 12.3 +(1 row) + +set enable_hashagg = true; +set enable_sort = false; +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + count +------- + 1006 +(1 row) + +set enable_sort = true; drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); ERROR: value 0 out of bounds for option "siglen" diff --git a/contrib/ltree/ltree--1.2--1.3.sql b/contrib/ltree/ltree--1.2--1.3.sql new file mode 100644 index 0000000000..8dd1b645c1 --- /dev/null +++ b/contrib/ltree/ltree--1.2--1.3.sql @@ -0,0 +1,23 @@ +/* contrib/ltree/ltree--1.2--1.3.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION ltree UPDATE TO '1.3'" to load this file. \quit + +CREATE FUNCTION ltree_hash(ltree) +RETURNS integer +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE FUNCTION ltree_hash_extended(ltree, bigint) +RETURNS bigint +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE OPERATOR CLASS hash_ltree_ops +DEFAULT FOR TYPE ltree USING hash +AS + OPERATOR 1 = , + FUNCTION 1 ltree_hash(ltree), + FUNCTION 2 ltree_hash_extended(ltree, bigint); + +ALTER OPERATOR =(ltree, ltree) SET (HASHES); diff --git a/contrib/ltree/ltree.control b/contrib/ltree/ltree.control index b408d64781..c2cbeda96c 100644 --- a/contrib/ltree/ltree.control +++ b/contrib/ltree/ltree.control @@ -1,6 +1,6 @@ # ltree extension comment = 'data type for hierarchical tree-like structures' -default_version = '1.2' +default_version = '1.3' module_pathname = '$libdir/ltree' relocatable = true trusted = true diff --git a/contrib/ltree/ltree_op.c b/contrib/ltree/ltree_op.c index da1db5fcd2..4199c70f7f 100644 --- a/contrib/ltree/ltree_op.c +++ b/contrib/ltree/ltree_op.c @@ -9,6 +9,7 @@ #include "access/htup_details.h" #include "catalog/pg_statistic.h" +#include "common/hashfn.h" #include "ltree.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -37,6 +38,8 @@ PG_FUNCTION_INFO_V1(lca); PG_FUNCTION_INFO_V1(ltree2text); PG_FUNCTION_INFO_V1(text2ltree); PG_FUNCTION_INFO_V1(ltreeparentsel); +PG_FUNCTION_INFO_V1(ltree_hash); +PG_FUNCTION_INFO_V1(ltree_hash_extended); int ltree_compare(const ltree *a, const ltree *b) @@ -588,3 +591,68 @@ ltreeparentsel(PG_FUNCTION_ARGS) PG_RETURN_FLOAT8((float8) selec); } + +/* + * Hashes the elements of the path using the same logic as hash_array + */ +Datum +ltree_hash(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + + uint32 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + while (an > 0) + { + uint32 levelHash = hash_any((unsigned char *) al->name, al->len); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT32(result); +} + + +/* + * Hashes the elements of the path using the same logic as hash_array_extended + * (and ltree_hash). It differs from hash_array_extended only in how it handles + * zero length label paths. We return 1 + seed to ensure that the low 32 bits + * of the result match ltree_hash when the seed is 0, as required by the hash + * index support functions, but to also return a different value when there is + * a seed. + */ +Datum +ltree_hash_extended(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + const uint64 seed = PG_GETARG_INT64(1); + + uint64 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + if (an == 0) + { + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result + seed); + } + + while (an > 0) + { + uint64 levelHash = hash_any_extended((unsigned char *) al->name, al->len, seed); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result); +} diff --git a/contrib/ltree/ltreetest.sql b/contrib/ltree/ltreetest.sql index d6996caf3c..388d5bb6f5 100644 --- a/contrib/ltree/ltreetest.sql +++ b/contrib/ltree/ltreetest.sql @@ -19,3 +19,4 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING gist(path); CREATE INDEX path_idx ON test USING btree(path); +CREATE INDEX path_hash_idx ON test USING hash(path); diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 402096f6c4..fa0b9dff91 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -282,9 +282,22 @@ SELECT ('{3456,1.2.3.4}'::ltree[] ?<@ '1.2.5') is null; SELECT '{ltree.asd, tree.awdfg}'::ltree[] ?@ 'tree & aWdfg@'::ltxtquery; SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; +-- Check that the ltree_hash() and ltree_hash_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, ltree_hash(v)::bit(32) as standard, + ltree_hash_extended(v, 0)::bit(32) as extended0, + ltree_hash_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE ltree_hash(v)::bit(32) != ltree_hash_extended(v, 0)::bit(32) + OR ltree_hash(v)::bit(32) = ltree_hash_extended(v, 1)::bit(32); + + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; SELECT * FROM ltreetest WHERE t <= '12.3' order by t asc; SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; @@ -328,6 +341,20 @@ SELECT * FROM ltreetest WHERE t ~ '23.*.1' order by t asc; SELECT * FROM ltreetest WHERE t ~ '23.*.2' order by t asc; SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; +drop index tstidx; +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; + +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + +set enable_hashagg = true; +set enable_sort = false; + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; +set enable_sort = true; + drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=2025)); diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml index 00a6ae70da..5c109d2d5f 100644 --- a/doc/src/sgml/ltree.sgml +++ b/doc/src/sgml/ltree.sgml @@ -623,6 +623,13 @@ Europe & Russia*@ & !Transportation <literal>>=</literal>, <literal>></literal> </para> </listitem> + <listitem> + <para> + Hash index over <type>ltree</type>: + <literal>=</literal> + </para> + </listitem> + <listitem> <para> GiST index over <type>ltree</type> (<literal>gist_ltree_ops</literal> @@ -712,6 +719,7 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING GIST (path); CREATE INDEX path_idx ON test USING BTREE (path); +CREATE INDEX path_hash_idx ON test USING HASH (path); </programlisting> <para> ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-11-29 01:37 jian he <[email protected]> parent: Tommy Pavlicek <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: jian he @ 2023-11-29 01:37 UTC (permalink / raw) To: Tommy Pavlicek <[email protected]>; +Cc: [email protected]; Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] On Wed, Nov 29, 2023 at 6:09 AM Tommy Pavlicek <[email protected]> wrote: > > On Thu, Jul 6, 2023 at 2:18 AM Daniel Gustafsson <[email protected]> wrote: > > > > > On 19 Jun 2023, at 11:18, Tommy Pavlicek <[email protected]> wrote: > > > > > Tommy, are you interested in extending ALTER OPERATOR to allow this, > > > which would also allow fixing the ltree operator? > > > > > > Yes, I can do that. I took a look over the code and email thread and it seems like it should be relatively straight forward. I'll put a patch together for that and then update this patch to alter the operator. > > > > Did you have a chance to look at this for an updated patch for this commitfest? > > I finally had a chance to look at this and I've updated the patch to > alter the = operator to enable hash joins. > > This is ready to be looked at now. > > Is there anything I need to do to move this forward? > you only change Makefile, you also need to change contrib/ltree/meson.build? +drop index tstidx; +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; + +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; Do you need to use EXPLAIN to demo the index usage? ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-12-01 00:44 Tommy Pavlicek <[email protected]> parent: jian he <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Tommy Pavlicek @ 2023-12-01 00:44 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: [email protected]; Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] On Tue, Nov 28, 2023 at 7:38 PM jian he <[email protected]> wrote: > you only change Makefile, you also need to change contrib/ltree/meson.build? > Do you need to use EXPLAIN to demo the index usage? Thanks! Yes, I missed the Meson build file. I added additional commands with EXPLAIN (COSTS OFF) as I found in other places. Patch updated for those comments (and a touch of cleanup in the tests) attached. Attachments: [application/octet-stream] 0001-ltree-hash-v3.patch (10.5K, ../../CAEhP-W8_N3seEhQVDhTxV17qn3qZsN6-xhc=Qn=AwLn+WmKWPA@mail.gmail.com/2-0001-ltree-hash-v3.patch) download | inline diff: diff --git a/contrib/ltree/Makefile b/contrib/ltree/Makefile index 770769a730..f50f2c9470 100644 --- a/contrib/ltree/Makefile +++ b/contrib/ltree/Makefile @@ -14,7 +14,7 @@ OBJS = \ ltxtquery_op.o EXTENSION = ltree -DATA = ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql +DATA = ltree--1.2--1.3.sql ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql PGFILEDESC = "ltree - hierarchical label data type" HEADERS = ltree.h diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index 984cd030cf..e0152e16fa 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1433,8 +1433,27 @@ SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; g.b.c.d.e (1 row) +-- Check that the ltree_hash() and ltree_hash_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, ltree_hash(v)::bit(32) as standard, + ltree_hash_extended(v, 0)::bit(32) as extended0, + ltree_hash_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE ltree_hash(v)::bit(32) != ltree_hash_extended(v, 0)::bit(32) + OR ltree_hash(v)::bit(32) = ltree_hash_extended(v, 1)::bit(32); + value | standard | extended0 | extended1 +-------+----------+-----------+----------- +(0 rows) + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + count +------- + 1006 +(1 row) + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; t ---------------------------------- @@ -7832,6 +7851,52 @@ SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; 23.3.32.21.5.14.10.17.1 (4 rows) +drop index tstidx; +--- test hash index +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +set enable_bitmapscan=off; +EXPLAIN (COSTS OFF) SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + QUERY PLAN +-------------------------------------- + Index Scan using tstidx on ltreetest + Index Cond: (t = '12.3'::ltree) +(2 rows) + +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + t +------ + 12.3 +(1 row) + +set enable_seqscan=on; +set enable_bitmapscan=on; +-- test hash aggregate +set enable_hashagg=on; +set enable_sort=off; +EXPLAIN (COSTS OFF) +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + QUERY PLAN +----------------------------------------------------- + Aggregate + -> HashAggregate + Group Key: ltreetest.t + -> Append + -> Seq Scan on ltreetest + -> Seq Scan on ltreetest ltreetest_1 +(6 rows) + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + count +------- + 1006 +(1 row) + +set enable_sort=on; drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); ERROR: value 0 out of bounds for option "siglen" diff --git a/contrib/ltree/ltree--1.2--1.3.sql b/contrib/ltree/ltree--1.2--1.3.sql new file mode 100644 index 0000000000..8dd1b645c1 --- /dev/null +++ b/contrib/ltree/ltree--1.2--1.3.sql @@ -0,0 +1,23 @@ +/* contrib/ltree/ltree--1.2--1.3.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION ltree UPDATE TO '1.3'" to load this file. \quit + +CREATE FUNCTION ltree_hash(ltree) +RETURNS integer +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE FUNCTION ltree_hash_extended(ltree, bigint) +RETURNS bigint +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE OPERATOR CLASS hash_ltree_ops +DEFAULT FOR TYPE ltree USING hash +AS + OPERATOR 1 = , + FUNCTION 1 ltree_hash(ltree), + FUNCTION 2 ltree_hash_extended(ltree, bigint); + +ALTER OPERATOR =(ltree, ltree) SET (HASHES); diff --git a/contrib/ltree/ltree.control b/contrib/ltree/ltree.control index b408d64781..c2cbeda96c 100644 --- a/contrib/ltree/ltree.control +++ b/contrib/ltree/ltree.control @@ -1,6 +1,6 @@ # ltree extension comment = 'data type for hierarchical tree-like structures' -default_version = '1.2' +default_version = '1.3' module_pathname = '$libdir/ltree' relocatable = true trusted = true diff --git a/contrib/ltree/ltree_op.c b/contrib/ltree/ltree_op.c index da1db5fcd2..4199c70f7f 100644 --- a/contrib/ltree/ltree_op.c +++ b/contrib/ltree/ltree_op.c @@ -9,6 +9,7 @@ #include "access/htup_details.h" #include "catalog/pg_statistic.h" +#include "common/hashfn.h" #include "ltree.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -37,6 +38,8 @@ PG_FUNCTION_INFO_V1(lca); PG_FUNCTION_INFO_V1(ltree2text); PG_FUNCTION_INFO_V1(text2ltree); PG_FUNCTION_INFO_V1(ltreeparentsel); +PG_FUNCTION_INFO_V1(ltree_hash); +PG_FUNCTION_INFO_V1(ltree_hash_extended); int ltree_compare(const ltree *a, const ltree *b) @@ -588,3 +591,68 @@ ltreeparentsel(PG_FUNCTION_ARGS) PG_RETURN_FLOAT8((float8) selec); } + +/* + * Hashes the elements of the path using the same logic as hash_array + */ +Datum +ltree_hash(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + + uint32 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + while (an > 0) + { + uint32 levelHash = hash_any((unsigned char *) al->name, al->len); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT32(result); +} + + +/* + * Hashes the elements of the path using the same logic as hash_array_extended + * (and ltree_hash). It differs from hash_array_extended only in how it handles + * zero length label paths. We return 1 + seed to ensure that the low 32 bits + * of the result match ltree_hash when the seed is 0, as required by the hash + * index support functions, but to also return a different value when there is + * a seed. + */ +Datum +ltree_hash_extended(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + const uint64 seed = PG_GETARG_INT64(1); + + uint64 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + if (an == 0) + { + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result + seed); + } + + while (an > 0) + { + uint64 levelHash = hash_any_extended((unsigned char *) al->name, al->len, seed); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result); +} diff --git a/contrib/ltree/ltreetest.sql b/contrib/ltree/ltreetest.sql index d6996caf3c..388d5bb6f5 100644 --- a/contrib/ltree/ltreetest.sql +++ b/contrib/ltree/ltreetest.sql @@ -19,3 +19,4 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING gist(path); CREATE INDEX path_idx ON test USING btree(path); +CREATE INDEX path_hash_idx ON test USING hash(path); diff --git a/contrib/ltree/meson.build b/contrib/ltree/meson.build index 44d0337e6e..e09642a18a 100644 --- a/contrib/ltree/meson.build +++ b/contrib/ltree/meson.build @@ -32,6 +32,7 @@ install_data( 'ltree--1.0--1.1.sql', 'ltree--1.1--1.2.sql', 'ltree--1.1.sql', + 'ltree--1.2--1.3.sql', kwargs: contrib_data_args, ) diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 402096f6c4..33db07d977 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -282,9 +282,22 @@ SELECT ('{3456,1.2.3.4}'::ltree[] ?<@ '1.2.5') is null; SELECT '{ltree.asd, tree.awdfg}'::ltree[] ?@ 'tree & aWdfg@'::ltxtquery; SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; +-- Check that the ltree_hash() and ltree_hash_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, ltree_hash(v)::bit(32) as standard, + ltree_hash_extended(v, 0)::bit(32) as extended0, + ltree_hash_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE ltree_hash(v)::bit(32) != ltree_hash_extended(v, 0)::bit(32) + OR ltree_hash(v)::bit(32) = ltree_hash_extended(v, 1)::bit(32); + + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; SELECT * FROM ltreetest WHERE t <= '12.3' order by t asc; SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; @@ -328,6 +341,36 @@ SELECT * FROM ltreetest WHERE t ~ '23.*.1' order by t asc; SELECT * FROM ltreetest WHERE t ~ '23.*.2' order by t asc; SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; +drop index tstidx; + +--- test hash index + +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +set enable_bitmapscan=off; + +EXPLAIN (COSTS OFF) SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + +set enable_seqscan=on; +set enable_bitmapscan=on; + +-- test hash aggregate + +set enable_hashagg=on; +set enable_sort=off; + +EXPLAIN (COSTS OFF) +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + +set enable_sort=on; + drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=2025)); diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml index 00a6ae70da..5c109d2d5f 100644 --- a/doc/src/sgml/ltree.sgml +++ b/doc/src/sgml/ltree.sgml @@ -623,6 +623,13 @@ Europe & Russia*@ & !Transportation <literal>>=</literal>, <literal>></literal> </para> </listitem> + <listitem> + <para> + Hash index over <type>ltree</type>: + <literal>=</literal> + </para> + </listitem> + <listitem> <para> GiST index over <type>ltree</type> (<literal>gist_ltree_ops</literal> @@ -712,6 +719,7 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING GIST (path); CREATE INDEX path_idx ON test USING BTREE (path); +CREATE INDEX path_hash_idx ON test USING HASH (path); </programlisting> <para> ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-12-04 06:46 jian he <[email protected]> parent: Tommy Pavlicek <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: jian he @ 2023-12-04 06:46 UTC (permalink / raw) To: Tommy Pavlicek <[email protected]>; +Cc: [email protected]; Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] On Fri, Dec 1, 2023 at 8:44 AM Tommy Pavlicek <[email protected]> wrote: > > > Patch updated for those comments (and a touch of cleanup in the tests) attached. it would be a better name as hash_ltree than ltree_hash, similar logic applies to ltree_hash_extended. that would be the convention. see: https://stackoverflow.com/a/69650940/15603477 Other than that, it looks good. ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2023-12-05 22:38 Tommy Pavlicek <[email protected]> parent: jian he <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: Tommy Pavlicek @ 2023-12-05 22:38 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: [email protected]; Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] Thanks. I've attached the latest version that updates the naming in line with the convention. On Mon, Dec 4, 2023 at 12:46 AM jian he <[email protected]> wrote: > > On Fri, Dec 1, 2023 at 8:44 AM Tommy Pavlicek <[email protected]> wrote: > > > > > > Patch updated for those comments (and a touch of cleanup in the tests) attached. > > it would be a better name as hash_ltree than ltree_hash, similar logic > applies to ltree_hash_extended. > that would be the convention. see: https://stackoverflow.com/a/69650940/15603477 > > > Other than that, it looks good. Attachments: [application/octet-stream] 0001-ltree-hash-v4.patch (10.5K, ../../CAEhP-W_Fbxw4o8ccN22gcfktqV2yq7Bz6VfubKLJfRxWWW_aAw@mail.gmail.com/2-0001-ltree-hash-v4.patch) download | inline diff: diff --git a/contrib/ltree/Makefile b/contrib/ltree/Makefile index 770769a730..f50f2c9470 100644 --- a/contrib/ltree/Makefile +++ b/contrib/ltree/Makefile @@ -14,7 +14,7 @@ OBJS = \ ltxtquery_op.o EXTENSION = ltree -DATA = ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql +DATA = ltree--1.2--1.3.sql ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql PGFILEDESC = "ltree - hierarchical label data type" HEADERS = ltree.h diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index 984cd030cf..e8b091d8fe 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1433,8 +1433,27 @@ SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; g.b.c.d.e (1 row) +-- Check that the hash_ltree() and hash_ltree_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, hash_ltree(v)::bit(32) as standard, + hash_ltree_extended(v, 0)::bit(32) as extended0, + hash_ltree_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32) + OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32); + value | standard | extended0 | extended1 +-------+----------+-----------+----------- +(0 rows) + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + count +------- + 1006 +(1 row) + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; t ---------------------------------- @@ -7832,6 +7851,52 @@ SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; 23.3.32.21.5.14.10.17.1 (4 rows) +drop index tstidx; +--- test hash index +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +set enable_bitmapscan=off; +EXPLAIN (COSTS OFF) SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + QUERY PLAN +-------------------------------------- + Index Scan using tstidx on ltreetest + Index Cond: (t = '12.3'::ltree) +(2 rows) + +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + t +------ + 12.3 +(1 row) + +set enable_seqscan=on; +set enable_bitmapscan=on; +-- test hash aggregate +set enable_hashagg=on; +set enable_sort=off; +EXPLAIN (COSTS OFF) +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + QUERY PLAN +----------------------------------------------------- + Aggregate + -> HashAggregate + Group Key: ltreetest.t + -> Append + -> Seq Scan on ltreetest + -> Seq Scan on ltreetest ltreetest_1 +(6 rows) + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + count +------- + 1006 +(1 row) + +set enable_sort=on; drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); ERROR: value 0 out of bounds for option "siglen" diff --git a/contrib/ltree/ltree--1.2--1.3.sql b/contrib/ltree/ltree--1.2--1.3.sql new file mode 100644 index 0000000000..bc9a34dd59 --- /dev/null +++ b/contrib/ltree/ltree--1.2--1.3.sql @@ -0,0 +1,23 @@ +/* contrib/ltree/ltree--1.2--1.3.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION ltree UPDATE TO '1.3'" to load this file. \quit + +CREATE FUNCTION hash_ltree(ltree) +RETURNS integer +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE FUNCTION hash_ltree_extended(ltree, bigint) +RETURNS bigint +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE OPERATOR CLASS hash_ltree_ops +DEFAULT FOR TYPE ltree USING hash +AS + OPERATOR 1 = , + FUNCTION 1 hash_ltree(ltree), + FUNCTION 2 hash_ltree_extended(ltree, bigint); + +ALTER OPERATOR =(ltree, ltree) SET (HASHES); diff --git a/contrib/ltree/ltree.control b/contrib/ltree/ltree.control index b408d64781..c2cbeda96c 100644 --- a/contrib/ltree/ltree.control +++ b/contrib/ltree/ltree.control @@ -1,6 +1,6 @@ # ltree extension comment = 'data type for hierarchical tree-like structures' -default_version = '1.2' +default_version = '1.3' module_pathname = '$libdir/ltree' relocatable = true trusted = true diff --git a/contrib/ltree/ltree_op.c b/contrib/ltree/ltree_op.c index da1db5fcd2..93043e1b91 100644 --- a/contrib/ltree/ltree_op.c +++ b/contrib/ltree/ltree_op.c @@ -9,6 +9,7 @@ #include "access/htup_details.h" #include "catalog/pg_statistic.h" +#include "common/hashfn.h" #include "ltree.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -37,6 +38,8 @@ PG_FUNCTION_INFO_V1(lca); PG_FUNCTION_INFO_V1(ltree2text); PG_FUNCTION_INFO_V1(text2ltree); PG_FUNCTION_INFO_V1(ltreeparentsel); +PG_FUNCTION_INFO_V1(hash_ltree); +PG_FUNCTION_INFO_V1(hash_ltree_extended); int ltree_compare(const ltree *a, const ltree *b) @@ -588,3 +591,68 @@ ltreeparentsel(PG_FUNCTION_ARGS) PG_RETURN_FLOAT8((float8) selec); } + +/* + * Hashes the elements of the path using the same logic as hash_array + */ +Datum +hash_ltree(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + + uint32 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + while (an > 0) + { + uint32 levelHash = hash_any((unsigned char *) al->name, al->len); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT32(result); +} + + +/* + * Hashes the elements of the path using the same logic as hash_array_extended + * (and hash_ltree). It differs from hash_array_extended only in how it handles + * zero length label paths. We return 1 + seed to ensure that the low 32 bits + * of the result match hash_ltree when the seed is 0, as required by the hash + * index support functions, but to also return a different value when there is + * a seed. + */ +Datum +hash_ltree_extended(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + const uint64 seed = PG_GETARG_INT64(1); + + uint64 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + if (an == 0) + { + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result + seed); + } + + while (an > 0) + { + uint64 levelHash = hash_any_extended((unsigned char *) al->name, al->len, seed); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result); +} diff --git a/contrib/ltree/ltreetest.sql b/contrib/ltree/ltreetest.sql index d6996caf3c..388d5bb6f5 100644 --- a/contrib/ltree/ltreetest.sql +++ b/contrib/ltree/ltreetest.sql @@ -19,3 +19,4 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING gist(path); CREATE INDEX path_idx ON test USING btree(path); +CREATE INDEX path_hash_idx ON test USING hash(path); diff --git a/contrib/ltree/meson.build b/contrib/ltree/meson.build index 44d0337e6e..e09642a18a 100644 --- a/contrib/ltree/meson.build +++ b/contrib/ltree/meson.build @@ -32,6 +32,7 @@ install_data( 'ltree--1.0--1.1.sql', 'ltree--1.1--1.2.sql', 'ltree--1.1.sql', + 'ltree--1.2--1.3.sql', kwargs: contrib_data_args, ) diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 402096f6c4..d0f2b906a5 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -282,9 +282,22 @@ SELECT ('{3456,1.2.3.4}'::ltree[] ?<@ '1.2.5') is null; SELECT '{ltree.asd, tree.awdfg}'::ltree[] ?@ 'tree & aWdfg@'::ltxtquery; SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; +-- Check that the hash_ltree() and hash_ltree_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, hash_ltree(v)::bit(32) as standard, + hash_ltree_extended(v, 0)::bit(32) as extended0, + hash_ltree_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32) + OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32); + + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; SELECT * FROM ltreetest WHERE t <= '12.3' order by t asc; SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; @@ -328,6 +341,36 @@ SELECT * FROM ltreetest WHERE t ~ '23.*.1' order by t asc; SELECT * FROM ltreetest WHERE t ~ '23.*.2' order by t asc; SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; +drop index tstidx; + +--- test hash index + +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +set enable_bitmapscan=off; + +EXPLAIN (COSTS OFF) SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + +set enable_seqscan=on; +set enable_bitmapscan=on; + +-- test hash aggregate + +set enable_hashagg=on; +set enable_sort=off; + +EXPLAIN (COSTS OFF) +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + +set enable_sort=on; + drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=2025)); diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml index 00a6ae70da..5c109d2d5f 100644 --- a/doc/src/sgml/ltree.sgml +++ b/doc/src/sgml/ltree.sgml @@ -623,6 +623,13 @@ Europe & Russia*@ & !Transportation <literal>>=</literal>, <literal>></literal> </para> </listitem> + <listitem> + <para> + Hash index over <type>ltree</type>: + <literal>=</literal> + </para> + </listitem> + <listitem> <para> GiST index over <type>ltree</type> (<literal>gist_ltree_ops</literal> @@ -712,6 +719,7 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING GIST (path); CREATE INDEX path_idx ON test USING BTREE (path); +CREATE INDEX path_hash_idx ON test USING HASH (path); </programlisting> <para> ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2024-02-01 15:11 vignesh C <[email protected]> parent: Tommy Pavlicek <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: vignesh C @ 2024-02-01 15:11 UTC (permalink / raw) To: Tommy Pavlicek <[email protected]>; +Cc: jian he <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] On Wed, 6 Dec 2023 at 04:08, Tommy Pavlicek <[email protected]> wrote: > > Thanks. > > I've attached the latest version that updates the naming in line with > the convention. > > On Mon, Dec 4, 2023 at 12:46 AM jian he <[email protected]> wrote: > > > > On Fri, Dec 1, 2023 at 8:44 AM Tommy Pavlicek <[email protected]> wrote: > > > > > > > > > Patch updated for those comments (and a touch of cleanup in the tests) attached. > > > > it would be a better name as hash_ltree than ltree_hash, similar logic > > applies to ltree_hash_extended. > > that would be the convention. see: https://stackoverflow.com/a/69650940/15603477 > > > > > > Other than that, it looks good. CFBot shows one of the test is failing as in [1]: diff -U3 /tmp/cirrus-ci-build/contrib/ltree/expected/ltree.out /tmp/cirrus-ci-build/build-32/testrun/ltree/regress/results/ltree.out --- /tmp/cirrus-ci-build/contrib/ltree/expected/ltree.out 2024-01-31 15:18:42.893039599 +0000 +++ /tmp/cirrus-ci-build/build-32/testrun/ltree/regress/results/ltree.out 2024-01-31 15:23:25.309028749 +0000 @@ -1442,9 +1442,14 @@ ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32) OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32); - value | standard | extended0 | extended1 --------+----------+-----------+----------- -(0 rows) + value | standard | extended0 | extended1 +-------------+----------------------------------+----------------------------------+---------------------------------- + 0 | 10001010100010010000000000001011 | 01011001111001000100011001011011 | 01011001111001000100011010011111 + 0.1 | 10100000111110001010110001001110 | 00111100100010001100110111010101 | 00111100100010001101100011010101 + 0.1.2 | 01111000011100000101111101110100 | 10101110011101011000000011010111 | 10101110011101110010001111000011 + 0 | 10001010100010010000000000001011 | 01011001111001000100011001011011 | 01011001111001000100011010011111 + 0_asd.1_ASD | 01000010001010000000101001001101 | 00111100100010001100110111010101 | 00111100100010001101100011010101 +(5 rows) Please post an updated version for the same. [1] - https://api.cirrus-ci.com/v1/artifact/task/5572544858685440/testrun/build-32/testrun/ltree/regress/r... Regards, Vignesh ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2024-02-05 00:00 jian he <[email protected]> parent: vignesh C <[email protected]> 0 siblings, 1 reply; 13+ messages in thread From: jian he @ 2024-02-05 00:00 UTC (permalink / raw) To: vignesh C <[email protected]>; +Cc: Tommy Pavlicek <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; Tom Lane <[email protected]>; [email protected] On Thu, Feb 1, 2024 at 11:11 PM vignesh C <[email protected]> wrote: > > On Wed, 6 Dec 2023 at 04:08, Tommy Pavlicek <[email protected]> wrote: > > > > Thanks. > > > > I've attached the latest version that updates the naming in line with > > the convention. > > > > On Mon, Dec 4, 2023 at 12:46 AM jian he <[email protected]> wrote: > > > > > > On Fri, Dec 1, 2023 at 8:44 AM Tommy Pavlicek <[email protected]> wrote: > > > > > > > > > > > > Patch updated for those comments (and a touch of cleanup in the tests) attached. > > > > > > it would be a better name as hash_ltree than ltree_hash, similar logic > > > applies to ltree_hash_extended. > > > that would be the convention. see: https://stackoverflow.com/a/69650940/15603477 > > > > > > > > > Other than that, it looks good. > > CFBot shows one of the test is failing as in [1]: > diff -U3 /tmp/cirrus-ci-build/contrib/ltree/expected/ltree.out > /tmp/cirrus-ci-build/build-32/testrun/ltree/regress/results/ltree.out > --- /tmp/cirrus-ci-build/contrib/ltree/expected/ltree.out 2024-01-31 > 15:18:42.893039599 +0000 > +++ /tmp/cirrus-ci-build/build-32/testrun/ltree/regress/results/ltree.out > 2024-01-31 15:23:25.309028749 +0000 > @@ -1442,9 +1442,14 @@ > ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) > WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32) > OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32); > - value | standard | extended0 | extended1 > --------+----------+-----------+----------- > -(0 rows) > + value | standard | > extended0 | extended1 > +-------------+----------------------------------+----------------------------------+---------------------------------- > + 0 | 10001010100010010000000000001011 | > 01011001111001000100011001011011 | 01011001111001000100011010011111 > + 0.1 | 10100000111110001010110001001110 | > 00111100100010001100110111010101 | 00111100100010001101100011010101 > + 0.1.2 | 01111000011100000101111101110100 | > 10101110011101011000000011010111 | 10101110011101110010001111000011 > + 0 | 10001010100010010000000000001011 | > 01011001111001000100011001011011 | 01011001111001000100011010011111 > + 0_asd.1_ASD | 01000010001010000000101001001101 | > 00111100100010001100110111010101 | 00111100100010001101100011010101 > +(5 rows) > > Please post an updated version for the same. > > [1] - https://api.cirrus-ci.com/v1/artifact/task/5572544858685440/testrun/build-32/testrun/ltree/regress/r... > It only fails on Linux - Debian Bullseye - Meson. I fixed the white space, named it v5. I also made the following changes: from uint64 levelHash = hash_any_extended((unsigned char *) al->name, al->len, seed); uint32 levelHash = hash_any((unsigned char *) al->name, al->len); to uint64 levelHash = DatumGetUInt64(hash_any_extended((unsigned char *) al->name, al->len, seed)); uint32 levelHash = DatumGetUInt32(hash_any((unsigned char *) al->name, al->len)); (these two line live in different functions) I have some problems testing it locally, so I post the patch. Attachments: [application/x-patch] v5-0001-add-hash-functions-for-the-ltree-extension.patch (11.3K, ../../CACJufxEjNrchox2FA1e2oCNTcgZeDDhCfyDhbC2+istv7FJiuQ@mail.gmail.com/2-v5-0001-add-hash-functions-for-the-ltree-extension.patch) download | inline diff: From ffc0e3daea5e490d8b9e6b7a0345f780d88b5624 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Fri, 2 Feb 2024 16:03:56 +0800 Subject: [PATCH v5 1/1] add hash functions for the ltree extension. --- contrib/ltree/Makefile | 2 +- contrib/ltree/expected/ltree.out | 65 +++++++++++++++++++++++++++++ contrib/ltree/ltree--1.2--1.3.sql | 23 +++++++++++ contrib/ltree/ltree.control | 2 +- contrib/ltree/ltree_op.c | 68 +++++++++++++++++++++++++++++++ contrib/ltree/ltreetest.sql | 1 + contrib/ltree/meson.build | 1 + contrib/ltree/sql/ltree.sql | 43 +++++++++++++++++++ doc/src/sgml/ltree.sgml | 8 ++++ 9 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 contrib/ltree/ltree--1.2--1.3.sql diff --git a/contrib/ltree/Makefile b/contrib/ltree/Makefile index 770769a7..f50f2c94 100644 --- a/contrib/ltree/Makefile +++ b/contrib/ltree/Makefile @@ -14,7 +14,7 @@ OBJS = \ ltxtquery_op.o EXTENSION = ltree -DATA = ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql +DATA = ltree--1.2--1.3.sql ltree--1.1--1.2.sql ltree--1.1.sql ltree--1.0--1.1.sql PGFILEDESC = "ltree - hierarchical label data type" HEADERS = ltree.h diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out index 984cd030..dd4f0fc1 100644 --- a/contrib/ltree/expected/ltree.out +++ b/contrib/ltree/expected/ltree.out @@ -1433,8 +1433,27 @@ SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; g.b.c.d.e (1 row) +-- Check that the hash_ltree() and hash_ltree_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, hash_ltree(v)::bit(32) as standard, + hash_ltree_extended(v, 0)::bit(32) as extended0, + hash_ltree_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32) + OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32); + value | standard | extended0 | extended1 +-------+----------+-----------+----------- +(0 rows) + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + count +------- + 1006 +(1 row) + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; t ---------------------------------- @@ -7832,6 +7851,52 @@ SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; 23.3.32.21.5.14.10.17.1 (4 rows) +drop index tstidx; +--- test hash index +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +set enable_bitmapscan=off; +EXPLAIN (COSTS OFF) SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + QUERY PLAN +-------------------------------------- + Index Scan using tstidx on ltreetest + Index Cond: (t = '12.3'::ltree) +(2 rows) + +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + t +------ + 12.3 +(1 row) + +set enable_seqscan=on; +set enable_bitmapscan=on; +-- test hash aggregate +set enable_hashagg=on; +set enable_sort=off; +EXPLAIN (COSTS OFF) +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + QUERY PLAN +----------------------------------------------------- + Aggregate + -> HashAggregate + Group Key: ltreetest.t + -> Append + -> Seq Scan on ltreetest + -> Seq Scan on ltreetest ltreetest_1 +(6 rows) + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + count +------- + 1006 +(1 row) + +set enable_sort=on; drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); ERROR: value 0 out of bounds for option "siglen" diff --git a/contrib/ltree/ltree--1.2--1.3.sql b/contrib/ltree/ltree--1.2--1.3.sql new file mode 100644 index 00000000..bc9a34dd --- /dev/null +++ b/contrib/ltree/ltree--1.2--1.3.sql @@ -0,0 +1,23 @@ +/* contrib/ltree/ltree--1.2--1.3.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION ltree UPDATE TO '1.3'" to load this file. \quit + +CREATE FUNCTION hash_ltree(ltree) +RETURNS integer +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE FUNCTION hash_ltree_extended(ltree, bigint) +RETURNS bigint +AS 'MODULE_PATHNAME' +LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE; + +CREATE OPERATOR CLASS hash_ltree_ops +DEFAULT FOR TYPE ltree USING hash +AS + OPERATOR 1 = , + FUNCTION 1 hash_ltree(ltree), + FUNCTION 2 hash_ltree_extended(ltree, bigint); + +ALTER OPERATOR =(ltree, ltree) SET (HASHES); diff --git a/contrib/ltree/ltree.control b/contrib/ltree/ltree.control index b408d647..c2cbeda9 100644 --- a/contrib/ltree/ltree.control +++ b/contrib/ltree/ltree.control @@ -1,6 +1,6 @@ # ltree extension comment = 'data type for hierarchical tree-like structures' -default_version = '1.2' +default_version = '1.3' module_pathname = '$libdir/ltree' relocatable = true trusted = true diff --git a/contrib/ltree/ltree_op.c b/contrib/ltree/ltree_op.c index da1db5fc..4c464153 100644 --- a/contrib/ltree/ltree_op.c +++ b/contrib/ltree/ltree_op.c @@ -9,6 +9,7 @@ #include "access/htup_details.h" #include "catalog/pg_statistic.h" +#include "common/hashfn.h" #include "ltree.h" #include "utils/builtins.h" #include "utils/lsyscache.h" @@ -37,6 +38,8 @@ PG_FUNCTION_INFO_V1(lca); PG_FUNCTION_INFO_V1(ltree2text); PG_FUNCTION_INFO_V1(text2ltree); PG_FUNCTION_INFO_V1(ltreeparentsel); +PG_FUNCTION_INFO_V1(hash_ltree); +PG_FUNCTION_INFO_V1(hash_ltree_extended); int ltree_compare(const ltree *a, const ltree *b) @@ -588,3 +591,68 @@ ltreeparentsel(PG_FUNCTION_ARGS) PG_RETURN_FLOAT8((float8) selec); } + +/* + * Hashes the elements of the path using the same logic as hash_array + */ +Datum +hash_ltree(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + + uint32 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + while (an > 0) + { + uint32 levelHash = DatumGetUInt32(hash_any((unsigned char *) al->name, al->len)); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT32(result); +} + + +/* + * Hashes the elements of the path using the same logic as hash_array_extended + * (and hash_ltree). It differs from hash_array_extended only in how it handles + * zero length label paths. We return 1 + seed to ensure that the low 32 bits + * of the result match hash_ltree when the seed is 0, as required by the hash + * index support functions, but to also return a different value when there is + * a seed. + */ +Datum +hash_ltree_extended(PG_FUNCTION_ARGS) +{ + ltree *a = PG_GETARG_LTREE_P(0); + const uint64 seed = PG_GETARG_INT64(1); + + uint64 result = 1; + int an = a->numlevel; + ltree_level *al = LTREE_FIRST(a); + + if (an == 0) + { + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result + seed); + } + + while (an > 0) + { + uint64 levelHash = DatumGetUInt64(hash_any_extended((unsigned char *) al->name, al->len, seed)); + + result = (result << 5) - result + levelHash; + + an--; + al = LEVEL_NEXT(al); + } + + PG_FREE_IF_COPY(a, 0); + PG_RETURN_UINT64(result); +} diff --git a/contrib/ltree/ltreetest.sql b/contrib/ltree/ltreetest.sql index d6996caf..388d5bb6 100644 --- a/contrib/ltree/ltreetest.sql +++ b/contrib/ltree/ltreetest.sql @@ -19,3 +19,4 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING gist(path); CREATE INDEX path_idx ON test USING btree(path); +CREATE INDEX path_hash_idx ON test USING hash(path); diff --git a/contrib/ltree/meson.build b/contrib/ltree/meson.build index 5862943e..29cc6865 100644 --- a/contrib/ltree/meson.build +++ b/contrib/ltree/meson.build @@ -32,6 +32,7 @@ install_data( 'ltree--1.0--1.1.sql', 'ltree--1.1--1.2.sql', 'ltree--1.1.sql', + 'ltree--1.2--1.3.sql', kwargs: contrib_data_args, ) diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql index 402096f6..869893db 100644 --- a/contrib/ltree/sql/ltree.sql +++ b/contrib/ltree/sql/ltree.sql @@ -282,9 +282,22 @@ SELECT ('{3456,1.2.3.4}'::ltree[] ?<@ '1.2.5') is null; SELECT '{ltree.asd, tree.awdfg}'::ltree[] ?@ 'tree & aWdfg@'::ltxtquery; SELECT '{j.k.l.m, g.b.c.d.e}'::ltree[] ?~ 'A*@|g.b.c.d.e'; +-- Check that the hash_ltree() and hash_ltree_extended() function's lower +-- 32 bits match when the seed is 0 and do not match when the seed != 0 +SELECT v as value, hash_ltree(v)::bit(32) as standard, + hash_ltree_extended(v, 0)::bit(32) as extended0, + hash_ltree_extended(v, 1)::bit(32) as extended1 +FROM (VALUES (NULL::ltree), (''::ltree), ('0'::ltree), ('0.1'::ltree), + ('0.1.2'::ltree), ('0'::ltree), ('0_asd.1_ASD'::ltree)) x(v) +WHERE hash_ltree(v)::bit(32) != hash_ltree_extended(v, 0)::bit(32) + OR hash_ltree(v)::bit(32) = hash_ltree_extended(v, 1)::bit(32); + + CREATE TABLE ltreetest (t ltree); \copy ltreetest FROM 'data/ltree.data' +SELECT count(*) from ltreetest; + SELECT * FROM ltreetest WHERE t < '12.3' order by t asc; SELECT * FROM ltreetest WHERE t <= '12.3' order by t asc; SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; @@ -328,6 +341,36 @@ SELECT * FROM ltreetest WHERE t ~ '23.*.1' order by t asc; SELECT * FROM ltreetest WHERE t ~ '23.*.2' order by t asc; SELECT * FROM ltreetest WHERE t ? '{23.*.1,23.*.2}' order by t asc; +drop index tstidx; + +--- test hash index + +create index tstidx on ltreetest using hash (t); +set enable_seqscan=off; +set enable_bitmapscan=off; + +EXPLAIN (COSTS OFF) SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; +SELECT * FROM ltreetest WHERE t = '12.3' order by t asc; + +set enable_seqscan=on; +set enable_bitmapscan=on; + +-- test hash aggregate + +set enable_hashagg=on; +set enable_sort=off; + +EXPLAIN (COSTS OFF) +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + +SELECT count(*) FROM ( +SELECT t FROM (SELECT * FROM ltreetest UNION ALL SELECT * FROM ltreetest) t1 GROUP BY t +) t2; + +set enable_sort=on; + drop index tstidx; create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=0)); create index tstidx on ltreetest using gist (t gist_ltree_ops(siglen=2025)); diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml index 00a6ae70..9584105b 100644 --- a/doc/src/sgml/ltree.sgml +++ b/doc/src/sgml/ltree.sgml @@ -623,6 +623,13 @@ Europe & Russia*@ & !Transportation <literal>>=</literal>, <literal>></literal> </para> </listitem> + <listitem> + <para> + Hash index over <type>ltree</type>: + <literal>=</literal> + </para> + </listitem> + <listitem> <para> GiST index over <type>ltree</type> (<literal>gist_ltree_ops</literal> @@ -712,6 +719,7 @@ INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Galaxies'); INSERT INTO test VALUES ('Top.Collections.Pictures.Astronomy.Astronauts'); CREATE INDEX path_gist_idx ON test USING GIST (path); CREATE INDEX path_idx ON test USING BTREE (path); +CREATE INDEX path_hash_idx ON test USING HASH (path); </programlisting> <para> -- 2.34.1 ^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [PATCH] ltree hash functions @ 2024-03-21 22:29 Tom Lane <[email protected]> parent: jian he <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Tom Lane @ 2024-03-21 22:29 UTC (permalink / raw) To: jian he <[email protected]>; +Cc: vignesh C <[email protected]>; Tommy Pavlicek <[email protected]>; [email protected]; Tomas Vondra <[email protected]>; [email protected] jian he <[email protected]> writes: > I also made the following changes: > from > uint64 levelHash = hash_any_extended((unsigned char *) al->name, al->len, seed); > uint32 levelHash = hash_any((unsigned char *) al->name, al->len); > to > uint64 levelHash = DatumGetUInt64(hash_any_extended((unsigned char *) > al->name, al->len, seed)); > uint32 levelHash = DatumGetUInt32(hash_any((unsigned char *) al->name, > al->len)); Yeah, that'd fail on 32-bit machines. Pushed v5 with some minor cosmetic tweaking. regards, tom lane ^ permalink raw reply [nested|flat] 13+ messages in thread
* [PATCH v3 2/4] pg_upgrade: bump minimum supported version to v10 @ 2026-04-17 18:20 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 13+ messages in thread From: Nathan Bossart @ 2026-04-17 18:20 UTC (permalink / raw) --- doc/src/sgml/ref/pgupgrade.sgml | 2 +- src/bin/pg_upgrade/check.c | 157 +---------------------- src/bin/pg_upgrade/controldata.c | 12 +- src/bin/pg_upgrade/exec.c | 23 ---- src/bin/pg_upgrade/file.c | 164 ------------------------- src/bin/pg_upgrade/multixact_rewrite.c | 11 +- src/bin/pg_upgrade/pg_upgrade.c | 31 +---- src/bin/pg_upgrade/pg_upgrade.h | 29 ----- src/bin/pg_upgrade/relfilenumber.c | 37 +----- src/bin/pg_upgrade/version.c | 127 ------------------- 10 files changed, 16 insertions(+), 577 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 38ca09b423c..c4ed75211db 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -67,7 +67,7 @@ PostgreSQL documentation </para> <para> - <application>pg_upgrade</application> supports upgrades from 9.2.X and later to the current + <application>pg_upgrade</application> supports upgrades from 10.X and later to the current major release of <productname>PostgreSQL</productname>, including snapshot and beta releases. </para> diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 5a7afe62eab..0813cef2729 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -28,7 +28,6 @@ static void check_for_incompatible_polymorphics(ClusterInfo *cluster); static void check_for_tables_with_oids(ClusterInfo *cluster); static void check_for_not_null_inheritance(ClusterInfo *cluster); static void check_for_gist_inet_ops(ClusterInfo *cluster); -static void check_for_pg_role_prefix(ClusterInfo *cluster); static void check_for_new_tablespace_dir(void); static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster); static void check_for_unicode_update(ClusterInfo *cluster); @@ -128,26 +127,6 @@ static DataTypesUsageChecks data_types_usage_checks[] = .threshold_version = ALL_VERSIONS }, - /* - * 9.3 -> 9.4 Fully implement the 'line' data type in 9.4, which - * previously returned "not enabled" by default and was only functionally - * enabled with a compile-time switch; as of 9.4 "line" has a different - * on-disk representation format. - */ - { - .status = gettext_noop("Checking for incompatible \"line\" data type"), - .report_filename = "tables_using_line.txt", - .base_query = - "SELECT 'pg_catalog.line'::pg_catalog.regtype AS oid", - .report_text = - gettext_noop("Your installation contains the \"line\" data type in user tables.\n" - "This data type changed its internal and input/output format\n" - "between your old and new versions so this\n" - "cluster cannot currently be upgraded. You can\n" - "drop the problem columns and restart the upgrade.\n"), - .threshold_version = 903 - }, - /* * pg_upgrade only preserves these system values: pg_class.oid pg_type.oid * pg_enum.oid @@ -209,30 +188,6 @@ static DataTypesUsageChecks data_types_usage_checks[] = .threshold_version = 1500 }, - /* - * It's no longer allowed to create tables or views with "unknown"-type - * columns. We do not complain about views with such columns, because - * they should get silently converted to "text" columns during the DDL - * dump and reload; it seems unlikely to be worth making users do that by - * hand. However, if there's a table with such a column, the DDL reload - * will fail, so we should pre-detect that rather than failing - * mid-upgrade. Worse, if there's a matview with such a column, the DDL - * reload will silently change it to "text" which won't match the on-disk - * storage (which is like "cstring"). So we *must* reject that. - */ - { - .status = gettext_noop("Checking for invalid \"unknown\" user columns"), - .report_filename = "tables_using_unknown.txt", - .base_query = - "SELECT 'pg_catalog.unknown'::pg_catalog.regtype AS oid", - .report_text = - gettext_noop("Your installation contains the \"unknown\" data type in user tables.\n" - "This data type is no longer allowed in tables, so this cluster\n" - "cannot currently be upgraded. You can drop the problem columns\n" - "and restart the upgrade.\n"), - .threshold_version = 906 - }, - /* * PG 12 changed the 'sql_identifier' type storage to be based on name, * not varchar, which breaks on-disk format for existing data. So we need @@ -255,23 +210,6 @@ static DataTypesUsageChecks data_types_usage_checks[] = .threshold_version = 1100 }, - /* - * JSONB changed its storage format during 9.4 beta, so check for it. - */ - { - .status = gettext_noop("Checking for incompatible \"jsonb\" data type in user tables"), - .report_filename = "tables_using_jsonb.txt", - .base_query = - "SELECT 'pg_catalog.jsonb'::pg_catalog.regtype AS oid", - .report_text = - gettext_noop("Your installation contains the \"jsonb\" data type in user tables.\n" - "The internal format of \"jsonb\" changed during 9.4 beta so this\n" - "cluster cannot currently be upgraded. You can drop the problem \n" - "columns and restart the upgrade.\n"), - .threshold_version = MANUAL_CHECK, - .version_hook = jsonb_9_4_check_applicable - }, - /* * PG 12 removed types abstime, reltime, tinterval. */ @@ -712,20 +650,6 @@ check_and_dump_old_cluster(void) if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1800) check_for_gist_inet_ops(&old_cluster); - /* - * Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged - * hash indexes - */ - if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906) - { - if (user_opts.check) - old_9_6_invalidate_hash_indexes(&old_cluster, true); - } - - /* 9.5 and below should not have roles starting with pg_ */ - if (GET_MAJOR_VERSION(old_cluster.major_version) <= 905) - check_for_pg_role_prefix(&old_cluster); - /* * While not a check option, we do this now because this is the only time * the old server is running. @@ -772,20 +696,6 @@ check_new_cluster(void) * system boundaries. */ check_hard_link(TRANSFER_MODE_SWAP); - - /* - * There are a few known issues with using --swap to upgrade from - * versions older than 10. For example, the sequence tuple format - * changed in v10, and the visibility map format changed in 9.6. - * While such problems are not insurmountable (and we may have to - * deal with similar problems in the future, anyway), it doesn't - * seem worth the effort to support swap mode for upgrades from - * long-unsupported versions. - */ - if (GET_MAJOR_VERSION(old_cluster.major_version) < 1000) - pg_fatal("Swap mode can only upgrade clusters from PostgreSQL version %s and later.", - "10"); - break; } @@ -831,10 +741,6 @@ issue_warnings_and_set_wal_level(void) */ start_postmaster(&new_cluster, true); - /* Reindex hash indexes for old < 10.0 */ - if (GET_MAJOR_VERSION(old_cluster.major_version) <= 906) - old_9_6_invalidate_hash_indexes(&new_cluster, false); - report_extension_updates(&new_cluster); stop_postmaster(false); @@ -892,9 +798,9 @@ check_cluster_versions(void) * upgrades */ - if (GET_MAJOR_VERSION(old_cluster.major_version) < 902) + if (GET_MAJOR_VERSION(old_cluster.major_version) < 10) pg_fatal("This utility can only upgrade from PostgreSQL version %s and later.", - "9.2"); + "10"); /* Only current PG version is supported as a target */ if (GET_MAJOR_VERSION(new_cluster.major_version) != GET_MAJOR_VERSION(PG_VERSION_NUM)) @@ -1569,12 +1475,10 @@ check_for_incompatible_polymorphics(ClusterInfo *cluster) ", 'array_cat(anyarray,anyarray)'" ", 'array_prepend(anyelement,anyarray)'"); - if (GET_MAJOR_VERSION(cluster->major_version) >= 903) appendPQExpBufferStr(&old_polymorphics, ", 'array_remove(anyarray,anyelement)'" ", 'array_replace(anyarray,anyelement,anyelement)'"); - if (GET_MAJOR_VERSION(cluster->major_version) >= 905) appendPQExpBufferStr(&old_polymorphics, ", 'array_position(anyarray,anyelement)'" ", 'array_position(anyarray,anyelement,integer)'" @@ -1870,63 +1774,6 @@ check_for_gist_inet_ops(ClusterInfo *cluster) check_ok(); } -/* - * check_for_pg_role_prefix() - * - * Versions older than 9.6 should not have any pg_* roles - */ -static void -check_for_pg_role_prefix(ClusterInfo *cluster) -{ - PGresult *res; - PGconn *conn = connectToServer(cluster, "template1"); - int ntups; - int i_roloid; - int i_rolname; - FILE *script = NULL; - char output_path[MAXPGPATH]; - - prep_status("Checking for roles starting with \"pg_\""); - - snprintf(output_path, sizeof(output_path), "%s/%s", - log_opts.basedir, - "pg_role_prefix.txt"); - - res = executeQueryOrDie(conn, - "SELECT oid AS roloid, rolname " - "FROM pg_catalog.pg_roles " - "WHERE rolname ~ '^pg_'"); - - ntups = PQntuples(res); - i_roloid = PQfnumber(res, "roloid"); - i_rolname = PQfnumber(res, "rolname"); - for (int rowno = 0; rowno < ntups; rowno++) - { - if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) - pg_fatal("could not open file \"%s\": %m", output_path); - fprintf(script, "%s (oid=%s)\n", - PQgetvalue(res, rowno, i_rolname), - PQgetvalue(res, rowno, i_roloid)); - } - - PQclear(res); - - PQfinish(conn); - - if (script) - { - fclose(script); - pg_log(PG_REPORT, "fatal"); - pg_fatal("Your installation contains roles starting with \"pg_\".\n" - "\"pg_\" is a reserved prefix for system roles. The cluster\n" - "cannot be upgraded until these roles are renamed.\n" - "A list of roles starting with \"pg_\" is in the file:\n" - " %s", output_path); - } - else - check_ok(); -} - /* * Callback function for processing results of query for * check_for_user_defined_encoding_conversions()'s UpgradeTask. If the query diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c index cffcd4b0eba..8188355240f 100644 --- a/src/bin/pg_upgrade/controldata.c +++ b/src/bin/pg_upgrade/controldata.c @@ -602,14 +602,12 @@ get_control_data(ClusterInfo *cluster) /* verify that we got all the mandatory pg_control data */ if (!got_xid || !got_oid || !got_multi || !got_oldestxid || - (!got_oldestmulti && - cluster->controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER) || + !got_oldestmulti || !got_mxoff || (!live_check && !got_nextxlogfile) || !got_float8_pass_by_value || !got_align || !got_blocksz || !got_largesz || !got_walsz || !got_walseg || !got_ident || !got_index || !got_toast || - (!got_large_object && - cluster->controldata.ctrl_ver >= LARGE_OBJECT_SIZE_PG_CONTROL_VER) || + !got_large_object || !got_date_is_int || !got_data_checksum_version || (!got_default_char_signedness && cluster->controldata.cat_ver >= DEFAULT_CHAR_SIGNEDNESS_CAT_VER)) @@ -630,8 +628,7 @@ get_control_data(ClusterInfo *cluster) if (!got_multi) pg_log(PG_REPORT, " latest checkpoint next MultiXactId"); - if (!got_oldestmulti && - cluster->controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER) + if (!got_oldestmulti) pg_log(PG_REPORT, " latest checkpoint oldest MultiXactId"); if (!got_oldestxid) @@ -670,8 +667,7 @@ get_control_data(ClusterInfo *cluster) if (!got_toast) pg_log(PG_REPORT, " maximum TOAST chunk size"); - if (!got_large_object && - cluster->controldata.ctrl_ver >= LARGE_OBJECT_SIZE_PG_CONTROL_VER) + if (!got_large_object) pg_log(PG_REPORT, " large-object chunk size"); if (!got_date_is_int) diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index e1de61f36ee..479557abdcc 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -55,16 +55,7 @@ get_bin_version(ClusterInfo *cluster) if (sscanf(cmd_output, "%*s %*s %d.%d", &v1, &v2) < 1) pg_fatal("could not get pg_ctl version output from %s", cmd); - if (v1 < 10) - { - /* old style, e.g. 9.6.1 */ - cluster->bin_version = v1 * 10000 + v2 * 100; - } - else - { - /* new style, e.g. 10.1 */ cluster->bin_version = v1 * 10000; - } } @@ -353,17 +344,7 @@ check_data_dir(ClusterInfo *cluster) check_single_dir(pg_data, "pg_subtrans"); check_single_dir(pg_data, PG_TBLSPC_DIR); check_single_dir(pg_data, "pg_twophase"); - - /* pg_xlog has been renamed to pg_wal in v10 */ - if (GET_MAJOR_VERSION(cluster->major_version) <= 906) - check_single_dir(pg_data, "pg_xlog"); - else check_single_dir(pg_data, "pg_wal"); - - /* pg_clog has been renamed to pg_xact in v10 */ - if (GET_MAJOR_VERSION(cluster->major_version) <= 906) - check_single_dir(pg_data, "pg_clog"); - else check_single_dir(pg_data, "pg_xact"); } @@ -404,10 +385,6 @@ check_bin_dir(ClusterInfo *cluster, bool check_versions) */ get_bin_version(cluster); - /* pg_resetxlog has been renamed to pg_resetwal in version 10 */ - if (GET_MAJOR_VERSION(cluster->bin_version) <= 906) - check_exec(cluster->bindir, "pg_resetxlog", check_versions); - else check_exec(cluster->bindir, "pg_resetwal", check_versions); if (cluster == &new_cluster) diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c index 5b276008614..af82c0de490 100644 --- a/src/bin/pg_upgrade/file.c +++ b/src/bin/pg_upgrade/file.c @@ -20,12 +20,8 @@ #include <linux/fs.h> #endif -#include "access/visibilitymapdefs.h" #include "common/file_perm.h" #include "pg_upgrade.h" -#include "storage/bufpage.h" -#include "storage/checksum.h" -#include "storage/checksum_impl.h" /* @@ -196,166 +192,6 @@ linkFile(const char *src, const char *dst, } -/* - * rewriteVisibilityMap() - * - * Transform a visibility map file, copying from src to dst. - * schemaName/relName are relation's SQL name (used for error messages only). - * - * In versions of PostgreSQL prior to catversion 201603011, PostgreSQL's - * visibility map included one bit per heap page; it now includes two. - * When upgrading a cluster from before that time to a current PostgreSQL - * version, we could refuse to copy visibility maps from the old cluster - * to the new cluster; the next VACUUM would recreate them, but at the - * price of scanning the entire table. So, instead, we rewrite the old - * visibility maps in the new format. That way, the all-visible bits - * remain set for the pages for which they were set previously. The - * all-frozen bits are never set by this conversion; we leave that to VACUUM. - */ -void -rewriteVisibilityMap(const char *fromfile, const char *tofile, - const char *schemaName, const char *relName) -{ - int src_fd; - int dst_fd; - PGIOAlignedBlock buffer; - PGIOAlignedBlock new_vmbuf; - ssize_t totalBytesRead = 0; - ssize_t src_filesize; - int rewriteVmBytesPerPage; - BlockNumber new_blkno = 0; - struct stat statbuf; - - /* Compute number of old-format bytes per new page */ - rewriteVmBytesPerPage = (BLCKSZ - SizeOfPageHeaderData) / 2; - - if ((src_fd = open(fromfile, O_RDONLY | PG_BINARY, 0)) < 0) - pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %m", - schemaName, relName, fromfile); - - if (fstat(src_fd, &statbuf) != 0) - pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %m", - schemaName, relName, fromfile); - - if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, - pg_file_create_mode)) < 0) - pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %m", - schemaName, relName, tofile); - - /* Save old file size */ - src_filesize = statbuf.st_size; - - /* - * Turn each visibility map page into 2 pages one by one. Each new page - * has the same page header as the old one. If the last section of the - * last page is empty, we skip it, mostly to avoid turning one-page - * visibility maps for small relations into two pages needlessly. - */ - while (totalBytesRead < src_filesize) - { - ssize_t bytesRead; - char *old_cur; - char *old_break; - char *old_blkend; - PageHeaderData pageheader; - bool old_lastblk; - - if ((bytesRead = read(src_fd, buffer.data, BLCKSZ)) != BLCKSZ) - { - if (bytesRead < 0) - pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %m", - schemaName, relName, fromfile); - else - pg_fatal("error while copying relation \"%s.%s\": partial page found in file \"%s\"", - schemaName, relName, fromfile); - } - - totalBytesRead += BLCKSZ; - old_lastblk = (totalBytesRead == src_filesize); - - /* Save the page header data */ - memcpy(&pageheader, buffer.data, SizeOfPageHeaderData); - - /* - * These old_* variables point to old visibility map page. old_cur - * points to current position on old page. old_blkend points to end of - * old block. old_break is the end+1 position on the old page for the - * data that will be transferred to the current new page. - */ - old_cur = buffer.data + SizeOfPageHeaderData; - old_blkend = buffer.data + bytesRead; - old_break = old_cur + rewriteVmBytesPerPage; - - while (old_break <= old_blkend) - { - char *new_cur; - bool empty = true; - bool old_lastpart; - - /* First, copy old page header to new page */ - memcpy(new_vmbuf.data, &pageheader, SizeOfPageHeaderData); - - /* Rewriting the last part of the last old page? */ - old_lastpart = old_lastblk && (old_break == old_blkend); - - new_cur = new_vmbuf.data + SizeOfPageHeaderData; - - /* Process old page bytes one by one, and turn it into new page. */ - while (old_cur < old_break) - { - uint8 byte = *(uint8 *) old_cur; - uint16 new_vmbits = 0; - int i; - - /* Generate new format bits while keeping old information */ - for (i = 0; i < BITS_PER_BYTE; i++) - { - if (byte & (1 << i)) - { - empty = false; - new_vmbits |= - VISIBILITYMAP_ALL_VISIBLE << (BITS_PER_HEAPBLOCK * i); - } - } - - /* Copy new visibility map bytes to new-format page */ - new_cur[0] = (char) (new_vmbits & 0xFF); - new_cur[1] = (char) (new_vmbits >> 8); - - old_cur++; - new_cur += BITS_PER_HEAPBLOCK; - } - - /* If the last part of the last page is empty, skip writing it */ - if (old_lastpart && empty) - break; - - /* Set new checksum for visibility map page, if enabled */ - if (new_cluster.controldata.data_checksum_version != PG_DATA_CHECKSUM_OFF) - ((PageHeader) new_vmbuf.data)->pd_checksum = - pg_checksum_page(new_vmbuf.data, new_blkno); - - errno = 0; - if (write(dst_fd, new_vmbuf.data, BLCKSZ) != BLCKSZ) - { - /* if write didn't set errno, assume problem is no disk space */ - if (errno == 0) - errno = ENOSPC; - pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %m", - schemaName, relName, tofile); - } - - /* Advance for next new page */ - old_break += rewriteVmBytesPerPage; - new_blkno++; - } - } - - /* Clean up */ - close(dst_fd); - close(src_fd); -} - void check_file_clone(void) { diff --git a/src/bin/pg_upgrade/multixact_rewrite.c b/src/bin/pg_upgrade/multixact_rewrite.c index 823984ec8f3..c45b3183684 100644 --- a/src/bin/pg_upgrade/multixact_rewrite.c +++ b/src/bin/pg_upgrade/multixact_rewrite.c @@ -25,10 +25,7 @@ static void RecordMultiXactMembers(SlruSegState *members_writer, * 32-bit offsets to the current format. * * Multixids in the range [from_multi, to_multi) are read from the old - * cluster, and written in the new format. An important edge case is that if - * from_multi == to_multi, this initializes the new pg_multixact files in the - * new format without trying to open any old files. (We rely on that when - * upgrading from PostgreSQL version 9.2 or below.) + * cluster, and written in the new format. * * Returns the new nextOffset value; the caller should set it in the new * control file. The new members always start from offset 1, regardless of @@ -42,6 +39,7 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi) SlruSegState *members_writer; char dir[MAXPGPATH] = {0}; bool prev_multixid_valid = false; + OldMultiXactReader *old_reader; /* * The range of valid multi XIDs is unchanged by the conversion (they are @@ -63,10 +61,6 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi) * Convert old multixids, if needed, by reading them one-by-one from the * old cluster. */ - if (to_multi != from_multi) - { - OldMultiXactReader *old_reader; - old_reader = AllocOldMultiXactRead(old_cluster.pgdata, old_cluster.controldata.chkpnt_nxtmulti, old_cluster.controldata.chkpnt_nxtmxoff); @@ -113,7 +107,6 @@ rewrite_multixacts(MultiXactId from_multi, MultiXactId to_multi) } FreeOldMultiXactReader(old_reader); - } /* Write the final 'next' offset to the last SLRU page */ RecordMultiXactOffset(offsets_writer, to_multi, diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 2127d297bfe..e5d7920c1b1 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -714,13 +714,6 @@ create_new_objects(void) end_progress_output(); check_ok(); - /* - * We don't have minmxids for databases or relations in pre-9.3 clusters, - * so set those after we have restored the schema. - */ - if (GET_MAJOR_VERSION(old_cluster.major_version) <= 902) - set_frozenxids(true); - /* update new_cluster info now that we have objects in the databases */ get_db_rel_and_slot_infos(&new_cluster); } @@ -777,10 +770,7 @@ copy_xact_xlog_xid(void) * Copy old commit logs to new data dir. pg_clog has been renamed to * pg_xact in post-10 clusters. */ - copy_subdir_files(GET_MAJOR_VERSION(old_cluster.major_version) <= 906 ? - "pg_clog" : "pg_xact", - GET_MAJOR_VERSION(new_cluster.major_version) <= 906 ? - "pg_clog" : "pg_xact"); + copy_subdir_files("pg_xact", "pg_xact"); prep_status("Setting oldest XID for new cluster"); exec_prog(UTILITY_LOG_FILE, NULL, true, true, @@ -809,7 +799,6 @@ copy_xact_xlog_xid(void) check_ok(); /* Copy or convert pg_multixact files */ - Assert(new_cluster.controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER); Assert(new_cluster.controldata.cat_ver >= MULTIXACTOFFSET_FORMATCHANGE_CAT_VER); if (old_cluster.controldata.cat_ver >= MULTIXACTOFFSET_FORMATCHANGE_CAT_VER) { @@ -844,25 +833,7 @@ copy_xact_xlog_xid(void) * Determine the range of multixacts to convert. */ nxtmulti = old_cluster.controldata.chkpnt_nxtmulti; - if (old_cluster.controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER) - { - /* Versions 9.3 - 18: convert all multixids */ oldstMulti = old_cluster.controldata.chkpnt_oldstMulti; - } - else - { - /* - * In PostgreSQL 9.2 and below, multitransactions were only used - * for row locking, and as such don't need to be preserved during - * upgrade. In that case, we utilize rewrite_multixacts() just to - * initialize new, empty files in the new format. - * - * It's important that the oldest multi is set to the latest value - * used by the old system, so that multixact.c returns the empty - * set for multis that might be present on disk. - */ - oldstMulti = nxtmulti; - } /* handle wraparound */ if (nxtmulti < FirstMultiXactId) nxtmulti = FirstMultiXactId; diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 1d767bbda2d..5a0d2045be5 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -101,19 +101,6 @@ extern char *output_files[]; #endif -/* - * The format of visibility map was changed with this 9.6 commit. - */ -#define VISIBILITY_MAP_FROZEN_BIT_CAT_VER 201603011 - -/* - * pg_multixact format changed in 9.3 commit 0ac5ad5134f2769ccbaefec73844f85, - * ("Improve concurrency of foreign key locking") which also updated catalog - * version to this value. pg_upgrade behavior depends on whether old and new - * server versions are both newer than this, or only the new one is. - */ -#define MULTIXACT_FORMATCHANGE_CAT_VER 201301231 - /* * MultiXactOffset was changed from 32-bit to 64-bit in version 19, at this * catalog version. pg_multixact files need to be converted when upgrading @@ -121,17 +108,6 @@ extern char *output_files[]; */ #define MULTIXACTOFFSET_FORMATCHANGE_CAT_VER 202512091 -/* - * large object chunk size added to pg_controldata, - * commit 5f93c37805e7485488480916b4585e098d3cc883 - */ -#define LARGE_OBJECT_SIZE_PG_CONTROL_VER 942 - -/* - * change in JSONB format during 9.4 beta - */ -#define JSONB_FORMAT_CHANGE_CAT_VER 201409291 - /* * The control file was changed to have the default char signedness, * commit 44fe30fdab6746a287163e7cc093fd36cda8eb92 @@ -429,8 +405,6 @@ void copyFileByRange(const char *src, const char *dst, const char *schemaName, const char *relName); void linkFile(const char *src, const char *dst, const char *schemaName, const char *relName); -void rewriteVisibilityMap(const char *fromfile, const char *tofile, - const char *schemaName, const char *relName); void check_file_clone(void); void check_copy_file_range(void); void check_hard_link(transferMode transfer_mode); @@ -500,10 +474,7 @@ unsigned int str2uint(const char *str); /* version.c */ -bool jsonb_9_4_check_applicable(ClusterInfo *cluster); bool protocol_negotiation_supported(const ClusterInfo *cluster); -void old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, - bool check_mode); void report_extension_updates(ClusterInfo *cluster); diff --git a/src/bin/pg_upgrade/relfilenumber.c b/src/bin/pg_upgrade/relfilenumber.c index d5088447e0d..ec2ff7acb21 100644 --- a/src/bin/pg_upgrade/relfilenumber.c +++ b/src/bin/pg_upgrade/relfilenumber.c @@ -18,7 +18,7 @@ #include "pg_upgrade.h" static void transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace, char *new_tablespace); -static void transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit); +static void transfer_relfile(FileNameMap *map, const char *type_suffix); /* * The following set of sync_queue_* functions are used for --swap to reduce @@ -496,25 +496,10 @@ transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace, char *new_tablespace) { int mapnum; - bool vm_must_add_frozenbit = false; - - /* - * Do we need to rewrite visibilitymap? - */ - if (old_cluster.controldata.cat_ver < VISIBILITY_MAP_FROZEN_BIT_CAT_VER && - new_cluster.controldata.cat_ver >= VISIBILITY_MAP_FROZEN_BIT_CAT_VER) - vm_must_add_frozenbit = true; /* --swap has its own subroutine */ if (user_opts.transfer_mode == TRANSFER_MODE_SWAP) { - /* - * We don't support --swap to upgrade from versions that require - * rewriting the visibility map. We should've failed already if - * someone tries to do that. - */ - Assert(!vm_must_add_frozenbit); - do_swap(maps, size, old_tablespace, new_tablespace); return; } @@ -525,13 +510,13 @@ transfer_single_new_db(FileNameMap *maps, int size, strcmp(maps[mapnum].old_tablespace, old_tablespace) == 0) { /* transfer primary file */ - transfer_relfile(&maps[mapnum], "", vm_must_add_frozenbit); + transfer_relfile(&maps[mapnum], ""); /* * Copy/link any fsm and vm files, if they exist */ - transfer_relfile(&maps[mapnum], "_fsm", vm_must_add_frozenbit); - transfer_relfile(&maps[mapnum], "_vm", vm_must_add_frozenbit); + transfer_relfile(&maps[mapnum], "_fsm"); + transfer_relfile(&maps[mapnum], "_vm"); } } } @@ -540,12 +525,10 @@ transfer_single_new_db(FileNameMap *maps, int size, /* * transfer_relfile() * - * Copy or link file from old cluster to new one. If vm_must_add_frozenbit - * is true, visibility map forks are converted and rewritten, even in link - * mode. + * Copy or link file from old cluster to new one. */ static void -transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit) +transfer_relfile(FileNameMap *map, const char *type_suffix) { char old_file[MAXPGPATH]; char new_file[MAXPGPATH]; @@ -604,14 +587,6 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro /* Copying files might take some time, so give feedback. */ pg_log(PG_STATUS, "%s", old_file); - if (vm_must_add_frozenbit && strcmp(type_suffix, "_vm") == 0) - { - /* Need to rewrite visibility map format */ - pg_log(PG_VERBOSE, "rewriting \"%s\" to \"%s\"", - old_file, new_file); - rewriteVisibilityMap(old_file, new_file, map->nspname, map->relname); - } - else switch (user_opts.transfer_mode) { case TRANSFER_MODE_CLONE: diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c index 047670d4acb..9e83d4659be 100644 --- a/src/bin/pg_upgrade/version.c +++ b/src/bin/pg_upgrade/version.c @@ -12,22 +12,6 @@ #include "fe_utils/string_utils.h" #include "pg_upgrade.h" -/* - * version_hook functions for check_for_data_types_usage in order to determine - * whether a data type check should be executed for the cluster in question or - * not. - */ -bool -jsonb_9_4_check_applicable(ClusterInfo *cluster) -{ - /* JSONB changed its storage format during 9.4 beta */ - if (GET_MAJOR_VERSION(cluster->major_version) == 904 && - cluster->controldata.cat_ver < JSONB_FORMAT_CHANGE_CAT_VER) - return true; - - return false; -} - /* * Older servers can't support newer protocol versions, so their connection * strings will need to lock max_protocol_version to 3.0. @@ -46,117 +30,6 @@ protocol_negotiation_supported(const ClusterInfo *cluster) return (GET_MAJOR_VERSION(cluster->major_version) >= 1100); } -/* - * old_9_6_invalidate_hash_indexes() - * 9.6 -> 10 - * Hash index binary format has changed from 9.6->10.0 - */ -void -old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool check_mode) -{ - int dbnum; - FILE *script = NULL; - bool found = false; - char *output_path = "reindex_hash.sql"; - - prep_status("Checking for hash indexes"); - - for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++) - { - PGresult *res; - bool db_used = false; - int ntups; - int rowno; - int i_nspname, - i_relname; - DbInfo *active_db = &cluster->dbarr.dbs[dbnum]; - PGconn *conn = connectToServer(cluster, active_db->db_name); - - /* find hash indexes */ - res = executeQueryOrDie(conn, - "SELECT n.nspname, c.relname " - "FROM pg_catalog.pg_class c, " - " pg_catalog.pg_index i, " - " pg_catalog.pg_am a, " - " pg_catalog.pg_namespace n " - "WHERE i.indexrelid = c.oid AND " - " c.relam = a.oid AND " - " c.relnamespace = n.oid AND " - " a.amname = 'hash'" - ); - - ntups = PQntuples(res); - i_nspname = PQfnumber(res, "nspname"); - i_relname = PQfnumber(res, "relname"); - for (rowno = 0; rowno < ntups; rowno++) - { - found = true; - if (!check_mode) - { - if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) - pg_fatal("could not open file \"%s\": %m", output_path); - if (!db_used) - { - PQExpBufferData connectbuf; - - initPQExpBuffer(&connectbuf); - appendPsqlMetaConnect(&connectbuf, active_db->db_name); - fputs(connectbuf.data, script); - termPQExpBuffer(&connectbuf); - db_used = true; - } - fprintf(script, "REINDEX INDEX %s.%s;\n", - quote_identifier(PQgetvalue(res, rowno, i_nspname)), - quote_identifier(PQgetvalue(res, rowno, i_relname))); - } - } - - PQclear(res); - - if (!check_mode && db_used) - { - /* mark hash indexes as invalid */ - PQclear(executeQueryOrDie(conn, - "UPDATE pg_catalog.pg_index i " - "SET indisvalid = false " - "FROM pg_catalog.pg_class c, " - " pg_catalog.pg_am a, " - " pg_catalog.pg_namespace n " - "WHERE i.indexrelid = c.oid AND " - " c.relam = a.oid AND " - " c.relnamespace = n.oid AND " - " a.amname = 'hash'")); - } - - PQfinish(conn); - } - - if (script) - fclose(script); - - if (found) - { - report_status(PG_WARNING, "warning"); - if (check_mode) - pg_log(PG_WARNING, "\n" - "Your installation contains hash indexes. These indexes have different\n" - "internal formats between your old and new clusters, so they must be\n" - "reindexed with the REINDEX command. After upgrading, you will be given\n" - "REINDEX instructions."); - else - pg_log(PG_WARNING, "\n" - "Your installation contains hash indexes. These indexes have different\n" - "internal formats between your old and new clusters, so they must be\n" - "reindexed with the REINDEX command. The file\n" - " %s\n" - "when executed by psql by the database superuser will recreate all invalid\n" - "indexes; until then, none of these indexes will be used.", - output_path); - } - else - check_ok(); -} - /* * Callback function for processing results of query for * report_extension_updates()'s UpgradeTask. If the query returned any rows, -- 2.50.1 (Apple Git-155) --6vzYk1swa63ey9i6 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v3-0003-psql-bump-minimum-supported-version-to-v10.patch ^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2026-04-17 18:20 UTC | newest] Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-06-17 18:19 Re: [PATCH] ltree hash functions Tom Lane <[email protected]> 2023-06-17 19:57 ` Tomas Vondra <[email protected]> 2023-06-19 09:18 ` Tommy Pavlicek <[email protected]> 2023-07-06 08:18 ` Daniel Gustafsson <[email protected]> 2023-11-28 22:08 ` Tommy Pavlicek <[email protected]> 2023-11-29 01:37 ` jian he <[email protected]> 2023-12-01 00:44 ` Tommy Pavlicek <[email protected]> 2023-12-04 06:46 ` jian he <[email protected]> 2023-12-05 22:38 ` Tommy Pavlicek <[email protected]> 2024-02-01 15:11 ` vignesh C <[email protected]> 2024-02-05 00:00 ` jian he <[email protected]> 2024-03-21 22:29 ` Tom Lane <[email protected]> 2026-04-17 18:20 [PATCH v3 2/4] pg_upgrade: bump minimum supported version to v10 Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox