Re: fix for BUG #3720: wrong results at using ltree

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>
Cc: Oleg Bartunov <obartunov(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Filip Rembiałkowski <filip(dot)rembialkowski(at)gmail(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>
Subject: Re: fix for BUG #3720: wrong results at using ltree
Date: 2019-07-17 18:33:46
Message-ID: CAPpHfdsvEaqXDsLWebQ4Q4GszRZ_P6h-1_cidmiqPMtWzXnmhA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Nikita,

On Tue, Jul 16, 2019 at 6:52 PM Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
> I looked at "ltree syntax improvement" patch and found two more very
> old bugs in ltree/lquery (fixes are attached):

Thank you for the fixes. I've couple notes on them.

0001-Fix-max-size-checking-for-ltree-and-lquery.patch

+#define LTREE_MAX_LEVELS Min(PG_UINT16_MAX, MaxAllocSize / sizeof(nodeitem))
+#define LQUERY_MAX_LEVELS Min(PG_UINT16_MAX, MaxAllocSize / ITEMSIZE)

Looks over caution. PG_UINT16_MAX is not even close to MaxAllocSize /
sizeof(nodeitem) or MaxAllocSize / ITEMSIZE.

0002-Fix-successive-lquery-ops.patch

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);

I'm not sure what do these checks do. Code around is uncommented and
puzzled. But could we guarantee the same invariant on the stage of
ltree/lquery parsing?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sergei Kornilov 2019-07-17 18:44:47 Re: Change ereport level for QueuePartitionConstraintValidation
Previous Message Andres Freund 2019-07-17 18:28:09 Re: refactoring - share str2*int64 functions