Hi,
Victor Yegorov reported a crash related to a GiST index on ltree [1],
following a pg_upgrade from 12.x to 14.x, with a data set reproducing
this. I spent some time investigating this, and it seems this is a silly
bug in commit
commit 911e70207703799605f5a0e8aad9f06cff067c63 (HEAD)
Author: Alexander Korotkov <akorotkov(at)postgresql(dot)org>
Date: Mon Mar 30 19:17:11 2020 +0300
Implement operator class parameters
...
in PG13, which modified ltree_gist so that siglen is opclass parameter
(and not hard-coded). But the procedures use LTREE_GET_ASIGLEN macro to
extract the value, which either extracts the value from the catalog (if
the index has opclass parameter) or uses a default value - but it always
uses LTREE_ASIGLEN_DEFAULT, which belongs to _ltree_gist opclass (i.e.
for array of ltree). And that's 28 instead of the 8, as it should be.
Attached is a simple patch tweaking the macros, which resolves the
issue. Maybe there should be a separate macro though, because "ASIGLEN"
probably refers to "array siglen".
But the bigger issue is this fixes indexes created before the upgrade,
but it breaks indexes created after it. Because those were created with
siglen 28B, so reverting to 8B breaks them. A bit of a symmetry :-/
I'm not sure what to do about this. There's no way to distinguish
indexes, and I think an index may actually be broken both with and
without the patch - imagine an index created before the upgrade (so
using siglen 8), but then receiving a couple new rows (siglen 28).
After thinking about this I only see two options:
1) Don't apply the patch and tell everyone using ltree_gist they need to
rebuild the index after pg_upgrade from 12 to 13+. The downside of this
is larger indexes (because some tuples are 20B larger).
2) Apply the patch + tell those who already upgraded from 12 to rebuild
ltree_gist indexes, because those might be broken due to new inserts.
My opinion is to do (2), because at least those who'll upgrade later
(which is a lot of people) will be fine without a rebuild. And it also
makes the indexes a bit smaller, thanks to saving 20B.
regards
[1]
https://www.postgresql.org/message-id/17406-71e02820ae79bb40%40postgresql.org
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL CompanyFrom c6c5471aebf49da96e47deb5286a6f7065068bd7 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.von...@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
