Re: remaining sql/json patches

On Fri, Jan 19, 2024 at 2:11 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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