From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew@dunslane.net>
cc: Amit Langote <amitlangote09@gmail.com>,
        Amit Langote <amitlan@postgresql.org>,
        pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Add basic JSON_TABLE() functionality
In-reply-to: <601996.1712244407@sss.pgh.pa.us>
References: <E1rsL9y-000S60-Pn@gemulon.postgresql.org>
 <f8402aef-c66c-4207-992c-fdee5e59c612@dunslane.net>
 <CA+HiwqF8KeHyoNi8TM-fUuE_002GYANbM55ybqEWi5QRinX5YQ@mail.gmail.com>
 <49c3a0cc-2998-4e25-8bc3-e70b3cba87eb@dunslane.net>
 <601996.1712244407@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 04 Apr 2024 11:26:47 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <603614.1712245382.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 04 Apr 2024 11:43:02 -0400
Message-ID: <603615.1712245382@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/603615.1712245382%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> Looking at parse.pl, I guess it's probably triggered by some
> unconventional use of ERRCODE_FEATURE_NOT_SUPPORTED in your
> gram.y changes.

Oh, no, it's parse.pl that is at fault.  It's doing this:

        if (/ERRCODE_FEATURE_NOT_SUPPORTED/)
        {
            $feature_not_supported =3D 1;
            next line;
        }
        ... later ...
        if ($feature_not_supported =3D=3D 1)
        {

            # we found an unsupported feature, but we have to
            # filter out ExecuteStmt: CREATE OptTemp TABLE ...
            # because the warning there is only valid in some situations
            if ($flds->[0] ne 'create' || $flds->[2] ne 'table')
            {
                add_to_buffer('rules',
                    'mmerror(PARSE_ERROR, ET_WARNING, "unsupported feature=
 will be passed to server");'
                );
            }
            $feature_not_supported =3D 0;
        }

In English, if a production in gram.y contains the string
ERRCODE_FEATURE_NOT_SUPPORTED anywhere, then the ecpg parser
will be made to spit out this warning when it reduces that
same production.

This works great for cases like

            | UNENCRYPTED PASSWORD Sconst
                {
                    ereport(ERROR,
                            (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                             errmsg("UNENCRYPTED PASSWORD is no longer sup=
ported"),
                             errhint("Remove UNENCRYPTED to store the pass=
word in encrypted form instead."),
                             parser_errposition(@1)));
                }

but not well at all if the error is conditional, as it is in the
json_table production.  The cowboy hack for CREATE OptTemp TABLE
was evidently meant to suppress what must have been the only
conditional FEATURE_NOT_SUPPORTED rule at the time, but it doesn't
look like that matches anything at all anymore.  If you look through
the generated preproc.y file you'll find quite a few of these that
are being reported inappropriately; I'm surprised that it hasn't
come up before.

We need to either nuke that ecpg warning entirely, or find a more
robust way of detecting whether the gram.y complaint is conditional.
I'll put a brown paper bag on my head before suggesting that maybe
we could pay attention to how far the errcode is indented.  Perhaps
a slightly nicer way is to assume it's conditional if any earlier
line in the rule starts with "if".

			regards, tom lane