Re: pgsql: Adjust string comparison in jsonpath

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Alexander Korotkov <akorotkov(at)postgresql(dot)org>, pgsql-committers <pgsql-committers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgsql: Adjust string comparison in jsonpath
Date: 2019-08-12 00:24:18
Message-ID: CA+hUKGJ8N5nK7awwAECgJk84x6c50NHE9nD7v43UgjHTD1hFgQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On Mon, Aug 12, 2019 at 10:30 AM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > > This appears to have upset prion when testing on en_US.iso885915.
> >
> > Also lapwing's "InstallCheck-fr_FR" stage crashed on this commit, when
> > running JSON queries, on HEAD and REL_12_STABLE:

> Thank you for pointing! I hope I can investigate this shortly.

Hi Alexander,

I can reproduce this by running LANG="fr_FR.ISO8859-1" initdb, then
running installcheck (on some other OSes that might be called just
"fr_FR"). See this comment in mbutils.c:

* The functions return a palloc'd, null-terminated string if conversion
* is required. However, if no conversion is performed, the given source
* string pointer is returned as-is.

You call pfree() on the result of pg_server_to_any() without checking
if it just returned in the input pointer (for example, it does that if
you give it an empty string). That triggers an assertion failure
somewhere inside pfree(). The following fixes that for me, and is
based on code I found elsewhere in the tree.

--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -2028,8 +2028,10 @@ compareStrings(const char *mbstr1, int mblen1,
cmp = binaryCompareStrings(utf8str1, strlen(utf8str1),

utf8str2, strlen(utf8str2));

- pfree(utf8str1);
- pfree(utf8str2);
+ if (mbstr1 != utf8str1)
+ pfree(utf8str1);
+ if (mbstr2 != utf8str2)
+ pfree(utf8str2);

With that fixed it no longer crashes, but then the regression test
fails due to differences in the output, which look like locale
ordering differences.

--
Thomas Munro
https://enterprisedb.com

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Alexander Korotkov 2019-08-12 03:27:38 pgsql: Fix string comparison in jsonpath
Previous Message Tom Lane 2019-08-11 22:55:38 pgsql: Partially revert "Insert temporary debugging output in regressio