pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Victor Yegorov <vyegorov@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Cc: Alexander Korotkov <aekorotkov@gmail.com>
Subject: Re: BUG #17406: Segmentation fault on GiST index after 14.2 upgrade
Date: Thu, 24 Feb 2022 19:36:40 +0100
Message-ID: <5cd74597-f555-d4e1-4dc2-d399180bb8cc@enterprisedb.com> (raw)
In-Reply-To: <14ac4567-3285-f549-67df-debaeab7719f@enterprisedb.com>
References: <17406-71e02820ae79bb40@postgresql.org>
	<cb3a777a-99c7-f671-532d-b828a2223502@enterprisedb.com>
	<CAGnEbojzf-uObyWeu1MPNKaYsnL_U2kg48K6Ec43waPRBu3DrQ@mail.gmail.com>
	<788a0a51-b1e3-3601-5783-3f6488796ec4@enterprisedb.com>
	<CAGnEboiQ-cnhXceFa9WfXk7y06fuVVsf9WcmEdPerjqth8QJxA@mail.gmail.com>
	<c733dae0-15ba-bcdd-e465-1a9f2fdefebf@enterprisedb.com>
	<e544c795-2c7a-e662-19d2-e34da12bfa66@enterprisedb.com>
	<14ac4567-3285-f549-67df-debaeab7719f@enterprisedb.com>

On 2/24/22 16:18, Tomas Vondra wrote:
> On 2/23/22 16:41, Tomas Vondra wrote:
>> On 2/23/22 16:36, Tomas Vondra wrote:
>>>>
>>>> I got approval to send a table with its data and index in the subject
>>>> datafile.
>>>> Taken from the v14 database.
>>>>
>>>
>>> Thanks for the data! I've been able to reproduce the issue (load into
>>> 12, pg_upgrade to 14 and run the query). After bisecting this for a
>>> while, this seems like a bug in PG13 commit
>>>
>>> commit 911e70207703799605f5a0e8aad9f06cff067c63 (HEAD)
>>> Author: Alexander Korotkov <akorotkov@postgresql.org>
>>> Date:   Mon Mar 30 19:17:11 2020 +0300
>>>
>>>     Implement operator class parameters
>>>
>>>     ...
>>>
>>> It works fine when upgrading to an earlier build, and crashes since this
>>> commit. I haven't investigated this further, but I guess there's some
>>> thinko in gist_ltree_ops, affecting how we interpret existing indexes.
>>>
>>> Alexander, any ideas?
>>>
>>
>> Sorry, I accidentally used Alexander's old address, so let me CC him
>> with the correct/current one.
>>
> 
> I tried investigating this a bit further, but my gist-foo is rather
> limited so I haven't made much progress so far. But as Victor already
> mentioned, REINDEX fixes this - so I tried comparing the broken index
> with a new one built on PG13.
> 
> And the it seems the indexes are mostly the same, with the exception of
> LSN in the page header. The only page that substantially differs is the
> first page, which is also storing less data (the broken version):
> 
> test=# SELECT * FROM
> page_header(get_raw_page('v3_region_copy_ltree_path_idx', 0));
>     lsn    | checksum | flags | lower | upper | special | pagesize | ...
> ----------+----------+-------+-------+-------+---------+----------+- ...
>  0/1CD80C8 |        0 |     0 |    40 |  7856 |    8176 |     8192 | ...
>   (1 row)
> 
> test=# SELECT * FROM
> page_header(get_raw_page('v3_region_copy_13_ltree_path_idx', 0));
>     lsn    | checksum | flags | lower | upper | special | pagesize | ...
> -----------+----------+-------+-------+-------+---------+----------+-...
>  0/7029760 |        0 |     0 |    40 |  7760 |    8176 |     8192 | ...
> (1 row)
> 

It took me a bit of digging and debugging, but I think I got it. The
problem is that to determine siglen value, some of the functions use

  LTREE_GET_ASIGLEN

macro, which inspects the opclass options and returns the value. But if
there are no explicit options for the particular index (as happens for
indexes created in older versions), it uses LTREE_ASIGLEN_DEFAULT, which
is 28 bytes. But that's wrong - this value for ltree[] gist opclass, not
for plain ltree - that uses just 8B. So we're off by 20B, which messes
all kinds of stuff, of course.

My guess this is a simple mistake, because the macro is in section
labeled as

  /* GiST support for ltree[] */

and the "A" in the name probably refers to "array" too.

Attached is a patch that fixes this, but I wonder what will happen to
new indexes created on 13+. Will check.

Victor, can you try with this patch applied?


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

  [text/x-patch] 0001-ltree-gist-siglen-fix.patch (5.3K, ../5cd74597-f555-d4e1-4dc2-d399180bb8cc@enterprisedb.com/2-0001-ltree-gist-siglen-fix.patch)
  download | inline diff:
From c6c5471aebf49da96e47deb5286a6f7065068bd7 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Thu, 24 Feb 2022 19:22:32 +0100
Subject: [PATCH] ltree gist siglen fix

---
 contrib/ltree/_ltree_gist.c | 12 ++++++------
 contrib/ltree/ltree.h       |  4 ++--
 contrib/ltree/ltree_gist.c  | 10 +++++-----
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/contrib/ltree/_ltree_gist.c b/contrib/ltree/_ltree_gist.c
index 72516c3b6b9..1f361416ac9 100644
--- a/contrib/ltree/_ltree_gist.c
+++ b/contrib/ltree/_ltree_gist.c
@@ -49,7 +49,7 @@ _ltree_compress(PG_FUNCTION_ARGS)
 {
 	GISTENTRY  *entry = (GISTENTRY *) PG_GETARG_POINTER(0);
 	GISTENTRY  *retval = entry;
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(LTREE_ASIGLEN_DEFAULT);
 
 	if (entry->leafkey)
 	{							/* ltree */
@@ -108,7 +108,7 @@ _ltree_same(PG_FUNCTION_ARGS)
 	ltree_gist *a = (ltree_gist *) PG_GETARG_POINTER(0);
 	ltree_gist *b = (ltree_gist *) PG_GETARG_POINTER(1);
 	bool	   *result = (bool *) PG_GETARG_POINTER(2);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(LTREE_ASIGLEN_DEFAULT);
 
 	if (LTG_ISALLTRUE(a) && LTG_ISALLTRUE(b))
 		*result = true;
@@ -154,7 +154,7 @@ _ltree_union(PG_FUNCTION_ARGS)
 {
 	GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0);
 	int		   *size = (int *) PG_GETARG_POINTER(1);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(LTREE_ASIGLEN_DEFAULT);
 	int32		i;
 	ltree_gist *result = ltree_gist_alloc(false, NULL, siglen, NULL, NULL);
 	BITVECP		base = LTG_SIGN(result);
@@ -219,7 +219,7 @@ _ltree_penalty(PG_FUNCTION_ARGS)
 	ltree_gist *origval = (ltree_gist *) DatumGetPointer(((GISTENTRY *) PG_GETARG_POINTER(0))->key);
 	ltree_gist *newval = (ltree_gist *) DatumGetPointer(((GISTENTRY *) PG_GETARG_POINTER(1))->key);
 	float	   *penalty = (float *) PG_GETARG_POINTER(2);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(LTREE_ASIGLEN_DEFAULT);
 
 	*penalty = hemdist(origval, newval, siglen);
 	PG_RETURN_POINTER(penalty);
@@ -242,7 +242,7 @@ _ltree_picksplit(PG_FUNCTION_ARGS)
 {
 	GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0);
 	GIST_SPLITVEC *v = (GIST_SPLITVEC *) PG_GETARG_POINTER(1);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(LTREE_ASIGLEN_DEFAULT);
 	OffsetNumber k,
 				j;
 	ltree_gist *datum_l,
@@ -508,7 +508,7 @@ _ltree_consistent(PG_FUNCTION_ARGS)
 
 	/* Oid		subtype = PG_GETARG_OID(3); */
 	bool	   *recheck = (bool *) PG_GETARG_POINTER(4);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(LTREE_ASIGLEN_DEFAULT);
 	ltree_gist *key = (ltree_gist *) DatumGetPointer(entry->key);
 	bool		res = false;
 
diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index 5b4be5e680a..c2ff15f72e6 100644
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -291,9 +291,9 @@ extern ltree_gist *ltree_gist_alloc(bool isalltrue, BITVECP sign, int siglen,
 
 #define LTREE_ASIGLEN_DEFAULT	(7 * sizeof(int32))
 #define LTREE_ASIGLEN_MAX		GISTMaxIndexKeySize
-#define LTREE_GET_ASIGLEN()		(PG_HAS_OPCLASS_OPTIONS() ? \
+#define LTREE_GET_ASIGLEN(siglen_default)		(PG_HAS_OPCLASS_OPTIONS() ? \
 								 ((LtreeGistOptions *) PG_GET_OPCLASS_OPTIONS())->siglen : \
-								 LTREE_ASIGLEN_DEFAULT)
+								 (siglen_default))
 #define ASIGLENBIT(siglen)		((siglen) * BITBYTE)
 
 #define ALOOPBYTE(siglen) \
diff --git a/contrib/ltree/ltree_gist.c b/contrib/ltree/ltree_gist.c
index 7c39ed4298e..e95c2f5d666 100644
--- a/contrib/ltree/ltree_gist.c
+++ b/contrib/ltree/ltree_gist.c
@@ -130,7 +130,7 @@ ltree_same(PG_FUNCTION_ARGS)
 	ltree_gist *a = (ltree_gist *) PG_GETARG_POINTER(0);
 	ltree_gist *b = (ltree_gist *) PG_GETARG_POINTER(1);
 	bool	   *result = (bool *) PG_GETARG_POINTER(2);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(SIGLEN_DEFAULT);
 
 	*result = false;
 	if (LTG_ISONENODE(a) != LTG_ISONENODE(b))
@@ -190,7 +190,7 @@ ltree_union(PG_FUNCTION_ARGS)
 {
 	GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0);
 	int		   *size = (int *) PG_GETARG_POINTER(1);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(SIGLEN_DEFAULT);
 	BITVECP		base = palloc0(siglen);
 	int32		i,
 				j;
@@ -260,7 +260,7 @@ ltree_penalty(PG_FUNCTION_ARGS)
 	ltree_gist *origval = (ltree_gist *) DatumGetPointer(((GISTENTRY *) PG_GETARG_POINTER(0))->key);
 	ltree_gist *newval = (ltree_gist *) DatumGetPointer(((GISTENTRY *) PG_GETARG_POINTER(1))->key);
 	float	   *penalty = (float *) PG_GETARG_POINTER(2);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(SIGLEN_DEFAULT);
 	int32		cmpr,
 				cmpl;
 
@@ -292,7 +292,7 @@ ltree_picksplit(PG_FUNCTION_ARGS)
 {
 	GistEntryVector *entryvec = (GistEntryVector *) PG_GETARG_POINTER(0);
 	GIST_SPLITVEC *v = (GIST_SPLITVEC *) PG_GETARG_POINTER(1);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(SIGLEN_DEFAULT);
 	OffsetNumber j;
 	int32		i;
 	RIX		   *array;
@@ -618,7 +618,7 @@ ltree_consistent(PG_FUNCTION_ARGS)
 
 	/* Oid		subtype = PG_GETARG_OID(3); */
 	bool	   *recheck = (bool *) PG_GETARG_POINTER(4);
-	int			siglen = LTREE_GET_ASIGLEN();
+	int			siglen = LTREE_GET_ASIGLEN(SIGLEN_DEFAULT);
 	ltree_gist *key = (ltree_gist *) DatumGetPointer(entry->key);
 	void	   *query = NULL;
 	bool		res = false;
-- 
2.34.1



view thread (15+ messages)  latest in thread

Message-ID: <5cd74597-f555-d4e1-4dc2-d399180bb8cc@enterprisedb.com>
Permalink:  ../5cd74597-f555-d4e1-4dc2-d399180bb8cc@enterprisedb.com/
Also on:    postgresql.org/message-id/5cd74597-f555-d4e1-4dc2-d399180bb8cc@enterprisedb.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: tomas.vondra@enterprisedb.com, vyegorov@gmail.com, pgsql-bugs@lists.postgresql.org, aekorotkov@gmail.com
  Subject: Re: BUG #17406: Segmentation fault on GiST index after 14.2 upgrade
  In-Reply-To: <5cd74597-f555-d4e1-4dc2-d399180bb8cc@enterprisedb.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox