Feedback-ID: ie0a040ee:Fastmail
Message-ID: <74e042f1-ea7b-4cd2-9ba0-87984e3bb947@eisentraut.org>
Date: Thu, 20 Mar 2025 15:18:07 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: SQL:2023 JSON simplified accessor support
To: Alexandra Wang <alexandra.wang.oss@gmail.com>,
 Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Matheus Alcantara <matheusssilv97@gmail.com>,
 Andrew Dunstan <andrew@dunslane.net>, Nikita Glukhov
 <glukhov.n.a@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 "David E. Wheeler" <david@justatheory.com>,
 jian he <jian.universality@gmail.com>
References: <d2d872c3-84f9-48f6-9963-f4537f02bc91@eisentraut.org>
 <adb15d59-2e6b-4d2e-ac5b-7aaea41f738f@gmail.com>
 <CAK98qZ2QGcyJrJAFv9wjY6S8yP9dUVnmG9Gb4OXuzuMMuM1Z5Q@mail.gmail.com>
 <b24dc997-d78f-47bd-b323-14e68232d127@dunslane.net>
 <21fa36b4-6d56-4556-b524-9e3955b922c8@eisentraut.org>
 <CAK98qZ3Ly6PhRwCVmMKJBba5oHVF9k370MMT2b_gep-SuQfRtg@mail.gmail.com>
 <CAK98qZ1za8XgOLY+2hQMPGoxVYFyh=dDkM2dZPVeJK4J6poyvA@mail.gmail.com>
 <CAK98qZ0EfrPcv3ZwGdeDiLPEpXYfHOEc8S2D=OCPJGcyWy5d-g@mail.gmail.com>
 <CAK98qZ1rZaVNy6ViangQom4iinZVH7=ebqhTsPxMQbN5ZtE9XQ@mail.gmail.com>
 <CAFY6G8cY5SUG5L-0ryVpom6HynE49p8-XQ59qkaEgnZZJ-c4Rg@mail.gmail.com>
 <CAFY6G8eUGcU3A4AHprgYbSOAOj7+WGhGxS_YP0wd2+aCpZTiNg@mail.gmail.com>
 <CAK98qZ14uKKRVFg4ibzMfReYaZD6Byxq8nYnvNNtXNjCfcd8kQ@mail.gmail.com>
 <CAHgHdKtL9nNaKXGCLt9gWugVzYWKhoBDQ7NESUwdCBty8kFK-A@mail.gmail.com>
 <CAK98qZ1nz6ZZhQqTOCNwRguZE5GsBLW5BQT_k=s7AA6gc2CN_g@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: 
 <CAK98qZ1nz6ZZhQqTOCNwRguZE5GsBLW5BQT_k=s7AA6gc2CN_g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/74e042f1-ea7b-4cd2-9ba0-87984e3bb947%40eisentraut.org>
Precedence: bulk

This patch set has expanded significantly in scope recently, which is 
probably the right thing, but that means there won't be enough time to 
review and finish it for PG18.  So I'm moving this to the next 
commitfest now.

On 13.03.25 15:02, Alexandra Wang wrote:
> Hi Mark,
> 
> Thank you so much for reviewing! I have attached the new patches.
> 
> On Tue, Mar 4, 2025 at 8:05 AM Mark Dilger <mark.dilger@enterprisedb.com 
> <mailto:mark.dilger@enterprisedb.com>> wrote:
> 
> 
>     On Mon, Mar 3, 2025 at 12:23 PM Alexandra Wang
>     <alexandra.wang.oss@gmail.com <mailto:alexandra.wang.oss@gmail.com>>
>     wrote:
> 
>           I've attached v10, which addresses your feedback.
> 
> 
>     Hi Alex!  Thanks for the patches.
> 
>     In src/test/regress/sql/jsonb.sql, the section marked with "--
>     slices are not supported" should be relabeled.  That comment
>     predates these patches, and is now misleading.
> 
>     A bit further down in expected/jsonb.out, there is an expected
>     failure, but no SQL comment to indicate that it is expected:
> 
>     +SELECT (t.jb).* FROM test_jsonb_dot_notation;
>     +ERROR:  missing FROM-clause entry for table "t"
>     +LINE 1: SELECT (t.jb).* FROM test_jsonb_dot_notation;
> 
>     Perhaps a "-- fails" comment would clarify?  Then, further down,
> 
> 
> Fixed.
> 
>     +SELECT (jb).a.**.x FROM test_jsonb_dot_notation; -- not supported
>     +ERROR:  syntax error at or near "**"
>     +LINE 1: SELECT (jb).a.**.x FROM test_jsonb_dot_notation;
> 
>     I wonder if it would be better to have the parser handle this case
>     and raise a ERRCODE_FEATURE_NOT_SUPPORTED instead?
> 
> 
> In 0008 I added a new token named "DOUBLE_ASTERISK" to the lexers to
> represent "**". Hope this helps!
> 
>     I got curious about the support for this new dot notation in the
>     plpgsql parser and tried:
> 
>     +DO $$
>     +DECLARE
>     +  a jsonb := '[1,2,3,4,5,6,7]'::jsonb;
>     +BEGIN
>     +  WHILE a IS NOT NULL
>     +  LOOP
>     +    RAISE NOTICE '%', a;
>     +    a := a[2:];
>     +  END LOOP;
>     +END
>     +$$ LANGUAGE plpgsql;
>     +NOTICE:  [1, 2, 3, 4, 5, 6, 7]
>     +NOTICE:  [3, 4, 5, 6, 7]
>     +NOTICE:  [5, 6, 7]
>     +NOTICE:  7
> 
>     which looks good!  But then I tried:
> 
>     +DO $$
>     +DECLARE
>     +  a jsonb := '{"": 6, "NU": [{"": [[3]]}, [6], [2], "bCi"], "aaf":
>     [-6, -8]}'::jsonb;
>     +BEGIN
>     +  WHILE a IS NOT NULL
>     +  LOOP
>     +    RAISE NOTICE '%', a;
>     +    a := COALESCE(a."NU", a[2]);
>     +  END LOOP;
>     +END
>     +$$ LANGUAGE plpgsql;
>     +NOTICE:  {"": 6, "NU": [{"": [[3]]}, [6], [2], "bCi"], "aaf": [-6, -8]}
>     +ERROR:  missing FROM-clause entry for table "a"
>     +LINE 1: a := COALESCE(a."NU", a[2])
>     +                      ^
>     +QUERY:  a := COALESCE(a."NU", a[2])
>     +CONTEXT:  PL/pgSQL function inline_code_block line 8 at assignment
> 
>     which suggests the plpgsql parser does not recognize a."NU" as we'd
>     expect.  Any thoughts on this?
> 
> 
> Thanks for the tests! I added them to the "jsonb" regress test.
> 
>     I notice there are no changes in src/interfaces/ecpg/test, which
>     concerns me.  The sqljson.pgc and sqljson_jsontable.pgc files are
>     already testing json handling in ecpg; perhaps just extend those a bit?
> 
> Thanks for bringing this up! I have added new tests in src/interfaces/ 
> ecpg/test/sql/sqljson.pgc.
> 
> 
> Best,
> Alex