pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Oleg Bartunov <obartunov@postgrespro.ru>
To: Thomas Munro <thomas.munro@gmail.com>
To: Pgsql Hackers <pgsql-hackers@postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Filip Rembiałkowski <filip.rembialkowski@gmail.com>
Cc: Teodor Sigaev <teodor@sigaev.ru>
Subject: Re: fix for BUG #3720: wrong results at using ltree
Date: Tue, 16 Jul 2019 18:50:22 +0300
Message-ID: <5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru> (raw )
In-Reply-To: <CAF4Au4yV8vsgA_7XCce4kFUa2BjseqWj=KVuOwcE+DGLj6AW7g@mail.gmail.com >
References: <CAP_rww=waX2Oo6q+MbMSiZ9ktdj6eaJj0cQzNu=Ry2cCDij5fw@mail.gmail.com >
<28639.1554565562@sss.pgh.pa.us >
<CA+hUKGKVbTGHGpHBX6EGew13CrfYcagThJqBAmeATCMKnVqisQ@mail.gmail.com >
<CAF4Au4yV8vsgA_7XCce4kFUa2BjseqWj=KVuOwcE+DGLj6AW7g@mail.gmail.com >
On 09.07.2019 17:57, Oleg Bartunov wrote:
> On Mon, Jul 8, 2019 at 7:22 AM Thomas Munro <thomas.munro@gmail.com> wrote:
>> On Sun, Apr 7, 2019 at 3:46 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> =?UTF-8?Q?Filip_Rembia=C5=82kowski?= <filip.rembialkowski@gmail.com> writes:
>>>> Here is my attempt to fix a 12-years old ltree bug (which is a todo item).
>>>> I see it's not backward-compatible, but in my understanding that's
>>>> what is documented. Previous behavior was inconsistent with
>>>> documentation (where single asterisk should match zero or more
>>>> labels).
>>>> http://archives.postgresql.org/pgsql-bugs/2007-11/msg00044.php
>> [...]
>>
>>> In short, I'm wondering if we should treat this as a documentation
>>> bug not a code bug. But to do that, we'd need a more accurate
>>> description of what the code is supposed to do, because the statement
>>> quoted above is certainly not a match to the actual behavior.
>> This patch doesn't apply. More importantly, it seems like we don't
>> have a consensus on whether we want it.
>>
>> Teodor, Oleg, would you like to offer an opinion here? If I
>> understand correctly, the choices are doc change, code/comment change
>> or WONT_FIX. This seems to be an entry that we can bring to a
>> conclusion in this CF with some input from the ltree experts.
> We are currently very busy and will look at the problem (and dig into
> our memory) later. There is also another ltree patch
> (https://commitfest.postgresql.org/23/1977/ ), it would be nice if
> Filip try it.
I looked at "ltree syntax improvement" patch and found two more very
old bugs in ltree/lquery (fixes are attached):
1. ltree/lquery level counter overflow is wrongly checked:
SELECT nlevel((repeat('a.', 65534) || 'a')::ltree);
nlevel
--------
65535
(1 row)
-- expected 65536 or error
SELECT nlevel((repeat('a.', 65535) || 'a')::ltree);
nlevel
--------
0
(1 row)
-- expected 65537 or error
SELECT nlevel((repeat('a.', 65536) || 'a')::ltree);
nlevel
--------
1
(1 row)
-- expected 'aaaaa...' or error
SELECT (repeat('a.', 65535) || 'a')::ltree;
ltree
-------
(1 row)
-- expected 'aaaaa...' or error
SELECT (repeat('a.', 65536) || 'a')::ltree;
ltree
-------
a
(1 row)
2. '*{a}.*{b}.*{c}' is not equivalent to '*{a+b+c}' (as I expect):
SELECT ltree '1.2' ~ '*{2}';
?column?
----------
t
(1 row)
-- expected true
SELECT ltree '1.2' ~ '*{1}.*{1}';
?column?
----------
f
(1 row)
Maybe these two bugs need a separate thread?
--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
[text/x-patch] 0001-Fix-max-size-checking-for-ltree-and-lquery.patch (3.7K, ../5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru/3-0001-Fix-max-size-checking-for-ltree-and-lquery.patch)
download | inline diff:
From 5334c92406fd281409594a8861cbb40ca9a8c0a9 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Date: Thu, 7 Mar 2019 17:45:44 +0300
Subject: [PATCH 1/2] Fix max size checking for ltree and lquery
---
contrib/ltree/expected/ltree.out | 16 ++++++++++++++++
contrib/ltree/ltree.h | 2 ++
contrib/ltree/ltree_io.c | 12 ++++++------
contrib/ltree/sql/ltree.sql | 5 +++++
4 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out
index 8226930..30b8247 100644
--- a/contrib/ltree/expected/ltree.out
+++ b/contrib/ltree/expected/ltree.out
@@ -457,6 +457,22 @@ SELECT nlevel('1.2.3.4');
4
(1 row)
+SELECT nlevel(('1' || repeat('.1', 65534))::ltree);
+ nlevel
+--------
+ 65535
+(1 row)
+
+SELECT nlevel(('1' || repeat('.1', 65535))::ltree);
+ERROR: number of ltree levels (65536) exceeds the maximum allowed (65535)
+SELECT ('1' || repeat('.1', 65534))::lquery IS NULL;
+ ?column?
+----------
+ f
+(1 row)
+
+SELECT ('1' || repeat('.1', 65535))::lquery IS NULL;
+ERROR: number of lquery levels (65536) exceeds the maximum allowed (65535)
SELECT '1.2'::ltree < '2.2'::ltree;
?column?
----------
diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index 366e580..0e843e3 100644
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -25,6 +25,7 @@ typedef struct
#define LTREE_HDRSIZE MAXALIGN( offsetof(ltree, data) )
#define LTREE_FIRST(x) ( (ltree_level*)( ((char*)(x))+LTREE_HDRSIZE ) )
+#define LTREE_MAX_LEVELS Min(PG_UINT16_MAX, MaxAllocSize / sizeof(nodeitem))
/* lquery */
@@ -77,6 +78,7 @@ typedef struct
#define LQUERY_HDRSIZE MAXALIGN( offsetof(lquery, data) )
#define LQUERY_FIRST(x) ( (lquery_level*)( ((char*)(x))+LQUERY_HDRSIZE ) )
+#define LQUERY_MAX_LEVELS Min(PG_UINT16_MAX, MaxAllocSize / ITEMSIZE)
#define LQUERY_HASNOT 0x01
diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.c
index f54f037..460ed1a 100644
--- a/contrib/ltree/ltree_io.c
+++ b/contrib/ltree/ltree_io.c
@@ -58,11 +58,11 @@ ltree_in(PG_FUNCTION_ARGS)
ptr += charlen;
}
- if (num + 1 > MaxAllocSize / sizeof(nodeitem))
+ if (num + 1 > LTREE_MAX_LEVELS)
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("number of levels (%d) exceeds the maximum allowed (%d)",
- num + 1, (int) (MaxAllocSize / sizeof(nodeitem)))));
+ errmsg("number of ltree levels (%d) exceeds the maximum allowed (%d)",
+ num + 1, (int) LTREE_MAX_LEVELS)));
list = lptr = (nodeitem *) palloc(sizeof(nodeitem) * (num + 1));
ptr = buf;
while (*ptr)
@@ -227,11 +227,11 @@ lquery_in(PG_FUNCTION_ARGS)
}
num++;
- if (num > MaxAllocSize / ITEMSIZE)
+ if (num > LQUERY_MAX_LEVELS)
ereport(ERROR,
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("number of levels (%d) exceeds the maximum allowed (%d)",
- num, (int) (MaxAllocSize / ITEMSIZE))));
+ errmsg("number of lquery levels (%d) exceeds the maximum allowed (%d)",
+ num, (int) LQUERY_MAX_LEVELS)));
curqlevel = tmpql = (lquery_level *) palloc0(ITEMSIZE * num);
ptr = buf;
while (*ptr)
diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql
index 846b04e..20b0928 100644
--- a/contrib/ltree/sql/ltree.sql
+++ b/contrib/ltree/sql/ltree.sql
@@ -90,6 +90,11 @@ SELECT '1.*.4|3|2.*{1}'::lquery;
SELECT 'qwerty%@*.tu'::lquery;
SELECT nlevel('1.2.3.4');
+SELECT nlevel(('1' || repeat('.1', 65534))::ltree);
+SELECT nlevel(('1' || repeat('.1', 65535))::ltree);
+SELECT ('1' || repeat('.1', 65534))::lquery IS NULL;
+SELECT ('1' || repeat('.1', 65535))::lquery IS NULL;
+
SELECT '1.2'::ltree < '2.2'::ltree;
SELECT '1.2'::ltree <= '2.2'::ltree;
SELECT '2.2'::ltree = '2.2'::ltree;
--
2.7.4
[text/x-patch] 0002-Fix-successive-lquery-ops.patch (2.5K, ../5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru/4-0002-Fix-successive-lquery-ops.patch)
download | inline diff:
From 073db1359564f1e9ce48ad553ce2c4d09a36d07c Mon Sep 17 00:00:00 2001
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
Date: Thu, 7 Mar 2019 16:47:10 +0300
Subject: [PATCH 2/2] Fix successive lquery * ops
---
contrib/ltree/expected/ltree.out | 24 ++++++++++++++++++++++++
contrib/ltree/lquery_op.c | 8 ++++----
contrib/ltree/sql/ltree.sql | 5 ++++-
3 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out
index 30b8247..b7f2ec6 100644
--- a/contrib/ltree/expected/ltree.out
+++ b/contrib/ltree/expected/ltree.out
@@ -929,6 +929,30 @@ SELECT 'a.b.c.d.e'::ltree ~ '*.!b.*.!c.*';
f
(1 row)
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{2}.*{2}';
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{2}.e';
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{4}';
+ ?column?
+----------
+ f
+(1 row)
+
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{5}.*';
+ ?column?
+----------
+ f
+(1 row)
+
SELECT 'QWER_TY'::ltree ~ 'q%@*';
?column?
----------
diff --git a/contrib/ltree/lquery_op.c b/contrib/ltree/lquery_op.c
index 62172d5..d4f4941 100644
--- a/contrib/ltree/lquery_op.c
+++ b/contrib/ltree/lquery_op.c
@@ -255,8 +255,8 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
}
else
{
- low_pos = cur_tpos + curq->low;
- high_pos = cur_tpos + curq->high;
+ low_pos = Min(low_pos + curq->low, PG_UINT16_MAX);
+ high_pos = Min(high_pos + curq->high, PG_UINT16_MAX);
if (ptr && ptr->q)
{
ptr->nq++;
@@ -282,8 +282,8 @@ checkCond(lquery_level *curq, int query_numlevel, ltree_level *curt, int tree_nu
}
else
{
- low_pos = cur_tpos + curq->low;
- high_pos = cur_tpos + curq->high;
+ low_pos = Min(low_pos + curq->low, PG_UINT16_MAX);
+ high_pos = Min(high_pos + curq->high, PG_UINT16_MAX);
}
curq = LQL_NEXT(curq);
diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql
index 20b0928..07cdf61 100644
--- a/contrib/ltree/sql/ltree.sql
+++ b/contrib/ltree/sql/ltree.sql
@@ -173,7 +173,10 @@ SELECT 'a.b.c.d.e'::ltree ~ 'a.!b.*{1}.!c.*';
SELECT 'a.b.c.d.e'::ltree ~ '!b.*{1}.!c.*';
SELECT 'a.b.c.d.e'::ltree ~ '*.!b.*{1}.!c.*';
SELECT 'a.b.c.d.e'::ltree ~ '*.!b.*.!c.*';
-
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{2}.*{2}';
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{2}.e';
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{1}.*{4}';
+SELECT 'a.b.c.d.e'::ltree ~ 'a.*{5}.*';
SELECT 'QWER_TY'::ltree ~ 'q%@*';
SELECT 'QWER_TY'::ltree ~ 'Q_t%@*';
--
2.7.4
view thread (25+ messages) latest in thread
Message-ID: <5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru>
Permalink: ../5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru/
Also on: postgresql.org/message-id/5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru
copy link · copy postgr.es · copy git am
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-hackers@postgresql.org
Cc: n.gluhov@postgrespro.ru, obartunov@postgrespro.ru, thomas.munro@gmail.com, tgl@sss.pgh.pa.us, filip.rembialkowski@gmail.com, teodor@sigaev.ru
Subject: Re: fix for BUG #3720: wrong results at using ltree
In-Reply-To: <5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru>
* 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