Re: remaining sql/json patches - Mailing list pgsql-hackers

From Amit Langote
Subject Re: remaining sql/json patches
Date
Msg-id CA+HiwqEZP-ybbvTZ_2gKJD2p60hvDkL9-UsoKrzirZkwdV60Ng@mail.gmail.com
Whole thread Raw
In response to Re: remaining sql/json patches  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: remaining sql/json patches
List pgsql-hackers
On Fri, Jan 19, 2024 at 2:11 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2024-Jan-18, Alvaro Herrera wrote:
>
> > commands/explain.c (Hmm, I think this is a preexisting bug actually)
> >
> >     3893          18 :         case T_TableFuncScan:
> >     3894          18 :             Assert(rte->rtekind == RTE_TABLEFUNC);
> >     3895          18 :             if (rte->tablefunc)
> >     3896           0 :                 if (rte->tablefunc->functype == TFT_XMLTABLE)
> >     3897           0 :                     objectname = "xmltable";
> >     3898             :                 else            /* Must be TFT_JSON_TABLE */
> >     3899           0 :                     objectname = "json_table";
> >     3900             :             else
> >     3901          18 :                 objectname = NULL;
> >     3902          18 :             objecttag = "Table Function Name";
> >     3903          18 :             break;
>
> Indeed -- the problem seems to be that add_rte_to_flat_rtable is
> creating a new RTE and zaps the ->tablefunc pointer for it.  So when
> EXPLAIN goes to examine the struct, there's a NULL pointer there and
> nothing is printed.

Ah yes.

> One simple fix is to change add_rte_to_flat_rtable so that it doesn't
> zero out the tablefunc pointer, but this is straight against what that
> function is trying to do, namely to remove substructure.

Yes.

>  Which means
> that we need to preserve the name somewhere else.  I added a new member
> to RangeTblEntry for this, which perhaps is a little ugly.  So here's
> the patch for that.
>
>  (I also added an alias to one XMLTABLE invocation
> under EXPLAIN, to show what it looks like when an alias is specified.
> Otherwise they're always shown as "XMLTABLE" "xmltable" which is a bit
> dumb).

Thanks for the patch.  Seems alright to me.

> Another possible way out is to decide that we don't want the
> "objectname" to be reported here.  I admit it's perhaps redundant.  In
> this case we'd just remove lines 3896-3899 shown above and let it be
> NULL.

Showing the function's name spelled out in the query (XMLTABLE /
JSON_TABLE) seems fine to me, even though maybe a bit redundant, yes.

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Andy Fan
Date:
Subject: Re: Strange Bitmapset manipulation in DiscreteKnapsack()
Next
From: "David G. Johnston"
Date:
Subject: Re: UUID v7