Re: patch: function xmltable

I found the whole TableExprGetTupleDesc() function a bit odd in
nodeFuncs.c, so I renamed it to ExecTypeFromTableExpr() and moved it to
execTuples.c -- but only because that's where ExecTypeFromTL and others
already live. I would have liked to move it to tupdesc.c instead, but
it requires knowledge of executor nodes, which is probably the reason
that ExecTypeFromTL is in execTuples. I think we'd eat that bit of
ugliness only because we're not the first. But anyway I quickly ran
into another problem.

I noticed that ExecTypeFromTableExpr is being called from the transform
phase, which is much earlier than the executor. I noticed because of
the warning that the above movement added to nodeFuncs.c,
src/backend/nodes/nodeFuncs.c:509:5: warning: implicit declaration of function 'ExecTypeFromTableExpr' [-Wimplicit-function-declaration]

so I thought, hm, is it okay to have parse analysis run an executor
function? (I suppose this is the reason you put it in nodeFuncs in the
first place). For fun, I tried this query under GDB, with a breakpoint
on exprTypmod():

SELECT X.*
FROM emp,
XMLTABLE ('//depts/dept/employee' passing doc
COLUMNS
empID INTEGER PATH '@id',
firstname int PATH 'name/first',
lastname VARCHAR(25) PATH 'name/last') AS X;

and sure enough, the type is resolved during parse analysis:

Breakpoint 1, exprTypmod (expr=expr(at)entry=0x1d23ad8)
at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
283 if (!expr)
(gdb) print *expr
$2 = {type = T_TableExpr}
(gdb) bt
#0 exprTypmod (expr=expr(at)entry=0x1d23ad8)
at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
#1 0x000000000080c500 in get_expr_result_type (expr=0x1d23ad8,
resultTypeId=0x7ffd482bfdb4, resultTupleDesc=0x7ffd482bfdb8)
at /pgsql/source/master/src/backend/utils/fmgr/funcapi.c:247
#2 0x000000000056de1b in expandRTE (rte=rte(at)entry=0x1d6b800, rtindex=2,
sublevels_up=0, location=location(at)entry=7,
include_dropped=include_dropped(at)entry=0 '\000',
colnames=colnames(at)entry=0x7ffd482bfe10, colvars=0x7ffd482bfe18)
at /pgsql/source/master/src/backend/parser/parse_relation.c:2052
#3 0x000000000056e131 in expandRelAttrs (pstate=pstate(at)entry=0x1d238a8,
rte=rte(at)entry=0x1d6b800, rtindex=<optimized out>,
sublevels_up=<optimized out>, location=location(at)entry=7)
at /pgsql/source/master/src/backend/parser/parse_relation.c:2435
#4 0x000000000056fa64 in ExpandSingleTable (pstate=pstate(at)entry=0x1d238a8,
rte=rte(at)entry=0x1d6b800, location=7,
make_target_entry=make_target_entry(at)entry=1 '\001')
at /pgsql/source/master/src/backend/parser/parse_target.c:1266
#5 0x000000000057135b in ExpandColumnRefStar (pstate=pstate(at)entry=0x1d238a8,
cref=0x1d22720, make_target_entry=make_target_entry(at)entry=1 '\001')
at /pgsql/source/master/src/backend/parser/parse_target.c:1158
#6 0x00000000005716f9 in transformTargetList (pstate=0x1d238a8,
targetlist=<optimized out>, exprKind=EXPR_KIND_SELECT_TARGET)

This seems fine I guess, and it seems to say that we ought to move the
code that generates the tupdesc to back parse analysis rather than
executor. Okay, fine. But let's find a better place than nodeFuncs.

But if I move the XMLTABLE() call to the target list instead, the type
is resolved at planner time:

SELECT
XMLTABLE ('/dept/employee' passing $$<dept bldg="114">
<employee id="903">
<name>
<first>Mary</first>
<last>Jones</last>
</name>
<office>415</office>
<phone>905-403-6112</phone>
<phone>647-504-4546</phone>
<salary currency="USD">64000</salary>
</employee>
</dept>$$
COLUMNS
empID INTEGER PATH '@id',
firstname varchar(4) PATH 'name/first',
lastname VARCHAR(25) PATH 'name/last') AS X;

Breakpoint 1, exprTypmod (expr=expr(at)entry=0x1d6bed8)
at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
283 if (!expr)
(gdb) bt
#0 exprTypmod (expr=expr(at)entry=0x1d6bed8)
at /pgsql/source/master/src/backend/nodes/nodeFuncs.c:283
#1 0x0000000000654058 in set_pathtarget_cost_width (root=0x1d6bc68,
target=0x1d6c728)
at /pgsql/source/master/src/backend/optimizer/path/costsize.c:4729
#2 0x000000000066c197 in grouping_planner (root=0x1d6bc68,
inheritance_update=40 '(', inheritance_update(at)entry=0 '\000',
tuple_fraction=0.01, tuple_fraction(at)entry=0)
at /pgsql/source/master/src/backend/optimizer/plan/planner.c:1745
#3 0x000000000066ef64 in subquery_planner (glob=glob(at)entry=0x1d6bbd0,
parse=parse(at)entry=0x1d23818, parent_root=parent_root(at)entry=0x0,
hasRecursion=hasRecursion(at)entry=0 '\000',
tuple_fraction=tuple_fraction(at)entry=0)
at /pgsql/source/master/src/backend/optimizer/plan/planner.c:795
#4 0x000000000066fe5e in standard_planner (parse=0x1d23818,
cursorOptions=256, boundParams=<optimized out>)
at /pgsql/source/master/src/backend/optimizer/plan/planner.c:307

This is surprising, but I'm not sure it's wrong.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services