Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hnPkY-0001sN-Ly for pgsql-hackers@arkaria.postgresql.org; Tue, 16 Jul 2019 15:52:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hnPkW-0007BP-A0 for pgsql-hackers@arkaria.postgresql.org; Tue, 16 Jul 2019 15:52:16 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hnPkV-00078L-Qz for pgsql-hackers@lists.postgresql.org; Tue, 16 Jul 2019 15:52:16 +0000 Received: from cyclops.postgrespro.ru ([93.174.131.138] helo=mail.postgrespro.ru) by magus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hnPkS-0004Ne-TY for pgsql-hackers@postgresql.org; Tue, 16 Jul 2019 15:52:15 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.postgrespro.ru (Postfix) with ESMTP id 522E821C1761; Tue, 16 Jul 2019 18:52:11 +0300 (MSK) X-Virus-Scanned: Debian amavisd-new at postgrespro.ru X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=x tagged_above=-99 required=4 WHITELISTED tests=[] autolearn=unavailable Received: from [10.100.236.30] (unknown [195.82.154.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id E470921C0711; Tue, 16 Jul 2019 18:52:10 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1563292331; bh=Fjk/3mx8SngE//aQmaiCoGJTapJgLN9387dt1rflGnE=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=QkSQ4eHFDiHUke06PsvRKI9gfzQesmM9nGQSFv3p8FtbH4Kbxxe07yX1RrEttn81h yv0s57l4aVC4G9+LQZwXXNqsU2rkUBixtPuXHEd+bvBR96+Ysu23Qr+QnSFIkbC7oe uBb5jFXvR2wkzUpj5pHdxFKCnwJeiFihVV4ciQiw= Subject: Re: fix for BUG #3720: wrong results at using ltree To: Oleg Bartunov , Thomas Munro , Pgsql Hackers Cc: Tom Lane , =?UTF-8?Q?Filip_Rembia=c5=82kowski?= , Teodor Sigaev References: <28639.1554565562@sss.pgh.pa.us> From: Nikita Glukhov Organization: Postgres Professional Message-ID: <5eedfd9e-2f14-a2df-3bd9-4c92eb0f08a5@postgrespro.ru> Date: Tue, 16 Jul 2019 18:50:22 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------4E850029013D08AFC18357A3" Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multi-part message in MIME format. --------------4E850029013D08AFC18357A3 Content-Type: multipart/alternative; boundary="------------F32ACCFB76857DAD74C07943" --------------F32ACCFB76857DAD74C07943 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 09.07.2019 17:57, Oleg Bartunov wrote: > On Mon, Jul 8, 2019 at 7:22 AM Thomas Munro wrote: >> On Sun, Apr 7, 2019 at 3:46 AM Tom Lane wrote: >>> =?UTF-8?Q?Filip_Rembia=C5=82kowski?= 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 --------------F32ACCFB76857DAD74C07943 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit


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
--------------F32ACCFB76857DAD74C07943-- --------------4E850029013D08AFC18357A3 Content-Type: text/x-patch; name="0001-Fix-max-size-checking-for-ltree-and-lquery.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Fix-max-size-checking-for-ltree-and-lquery.patch" From 5334c92406fd281409594a8861cbb40ca9a8c0a9 Mon Sep 17 00:00:00 2001 From: Nikita Glukhov 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 --------------4E850029013D08AFC18357A3 Content-Type: text/x-patch; name="0002-Fix-successive-lquery-ops.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-Fix-successive-lquery-ops.patch" From 073db1359564f1e9ce48ad553ce2c4d09a36d07c Mon Sep 17 00:00:00 2001 From: Nikita Glukhov 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 --------------4E850029013D08AFC18357A3--