Extending outfuncs support to utility statements
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Date: 2022-07-09T22:20:26Z
Lists: pgsql-hackers
Attachments
- enable-outfuncs-for-utility-stmts-wip.patch (text/x-diff) patch
We've long avoided building I/O support for utility-statement node
types, mainly because it didn't seem worth the trouble to write and
maintain such code by hand. Now that the automatic node-support-code
generation patch is in, that argument is gone, and it's just a matter
of whether the benefits are worth the backend code bloat. I can
see two benefits worth considering:
* Seems like having such support would be pretty useful for
debugging.
* The only reason struct Query still needs a handwritten output
function is that special logic is needed to prevent trying to
print the utilityStatement field when it's a utility statement
we lack outfuncs support for. Now it wouldn't be that hard
to get gen_node_support.pl to replicate that special logic,
and if we stick with the status-quo functionality then I think we
should do that so that we can get rid of the handwritten function.
But the other alternative is to provide outfuncs support for all
utility statements and drop the conditionality.
So I looked into how much code are we talking about. On my
RHEL8 x86_64 machine, the code sizes for outfuncs/readfuncs
as of HEAD are
$ size outfuncs.o readfuncs.o
text data bss dec hex filename
117173 0 0 117173 1c9b5 outfuncs.o
64540 0 0 64540 fc1c readfuncs.o
If we just open the floodgates and enable both outfuncs and
readfuncs support for all *Stmt nodes (plus some node types
that thereby become dumpable, like AlterTableCmd), then
this becomes
$ size outfuncs.o readfuncs.o
text data bss dec hex filename
139503 0 0 139503 220ef outfuncs.o
95562 0 0 95562 1754a readfuncs.o
For my taste, the circa 20K growth in outfuncs.o is an okay
price for being able to inspect utility statements more easily.
However, I'm less thrilled with the 30K growth in readfuncs.o,
because I can't see that we'd get any direct benefit from that.
So I think a realistic proposal is to enable outfuncs support
but keep readfuncs disabled. The attached WIP patch does that,
and gives me these code sizes:
$ size outfuncs.o readfuncs.o
text data bss dec hex filename
139503 0 0 139503 220ef outfuncs.o
69356 0 0 69356 10eec readfuncs.o
(The extra readfuncs space comes from not troubling over the
subsidiary node types such as AlterTableCmd. We could run
around and mark those no_read, but I didn't bother yet.)
The support-suppression code in gen_node_support.pl was a crude
hack before, and this patch doesn't make it any less so.
If we go this way, it would be better to move the knowledge that
we're suppressing read functionality into the utility statement
node declarations. We could just manually label them all
pg_node_attr(no_read), but what I'm kind of tempted to do is
invent a dummy abstract node type like Expr, and make all the
utility statements inherit from it:
typedef struct UtilityStmt
{
pg_node_attr(abstract, no_read)
NodeTag type;
} UtilityStmt;
Thoughts?
regards, tom lane
Commits
-
Avoid copying undefined data in _readA_Const().
- e060cd59fabd 16.0 landed
-
Enable WRITE_READ_PARSE_PLAN_TREES of rewritten utility statements
- 787102b56373 16.0 landed
-
Implement WRITE_READ_PARSE_PLAN_TREES for raw parse trees
- 40ad8f9deed2 16.0 landed
-
Don't lose precision for float fields of Nodes.
- acd624644bc4 16.0 landed
-
Fix write/read of empty string fields in Nodes.
- 8999f5ed3cd7 16.0 landed
-
Add read support for some missing raw parse nodes
- a6bc3301925e 16.0 landed
-
Fix reading of BitString nodes
- 2cb1a5a8d4ae 16.0 landed
-
Fix reading of most-negative integer value nodes
- 43f4b349152d 16.0 landed