Re: patch: function xmltable

2016-11-22 21:47 GMT+01:00 Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>:

> 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]
>

The tuple descriptor should not be serialized.

When xmltable is called directly, then living tuple descriptor is used -
created in transform time. Another situation is when xmltable is used from
view, where transform time is skipped.

Originally I serialized generated type - but I had the problems with record
types - the current infrastructure expects serialization only real types.

My solution is a recheck of tuple descriptor in executor time. It is small
overhead - once per query - but it allows use xmltable from views without
necessity to specify returned columns explicitly.

>
> 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.
>

There are different processing for Set Returning nodes called from
paramlist and from tablelist. In last case the invokes exprTypmod early.

There is a different case, that you didn't check

CREATE VIEW x AS SELECT xmltable(...)
CREATE VIEW x1 AS SELECT * FROM xmltable(...)

close session

and in new session
SELECT * FROM x;
SELECT * FROM x1;

Regards

Pavel

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