Re: Making CallContext and InlineCodeBlock less special-case-y

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Subject: Re: Making CallContext and InlineCodeBlock less special-case-y
Date: 2022-07-10 00:45:07
Message-ID: 86971.1657413907@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> As committed, gen_node_support.pl excludes CallContext and InlineCodeBlock
> from getting unneeded support functions via some very ad-hoc code.
> (Right now, there are some other node types that are handled similarly,
> but I'm looking to drive that set to empty.) After looking at the
> situation a bit, I think the problem is that these nodes are declared
> in parsenodes.h even though they have exactly nothing to do with
> parse trees. What they are is function-calling API infrastructure,
> so it seems like the most natural home for them is fmgr.h. A weaker
> case could be made for funcapi.h, perhaps.

On further thought, another way we could do this is to leave them where
they are but label them with a new attribute pg_node_attr(node_tag_only).
The big advantage of this idea is that it lets us explain
gen_node_support.pl's handling of execnodes.h and some other files as
"Nodes declared in these files are automatically assumed to be
node_tag_only. At some future date we might label them explicitly
and remove the file-level assumption." That gives us an easy fix
if we ever find ourselves wanting to supply support functions for
a subset of the nodes in one of those files.

This ties in a little bit with an idea I had for cleaning up the
other ad-hocery remaining in gen_node_support.pl. It looks like
we are heading towards marking all the raw-parse-tree nodes and
utility-statement nodes as no_read, so as to be able to support them
in outfuncs but not readfuncs. But if we're going to touch all of
those declarations, how about doing something a bit higher-level,
and marking them with semantic categories? That is,
"pg_node_attr(raw_parse_node)" if the node appears in raw parse
trees but not anywhere later in the pipeline, or
"pg_node_attr(utility_statement)" if that's what it is. Currently
these labels would just act as "no_read", but this approach would
make it a whole lot easier to change our minds later about how to
handle these categories of nodes.

I'm not entirely sure whether pg_node_attr(utility_statement) is
a better or worse idea than the inherit-from-UtilityStmt method
I posited in a nearby thread [1]. In principle we could do the
raw-parse-node labeling that way too, but for some reason it
doesn't seem quite as nice for raw parse nodes, mainly because
a subclass for them doesn't seem as well defined as one for
utility statements.

Thoughts?

regards, tom lane

[1] https://www.postgresql.org/message-id/4159834.1657405226%40sss.pgh.pa.us

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2022-07-10 00:53:31 Fix gcc warning in sync.c (usr/src/backend/storage/sync/sync.c)
Previous Message Tom Lane 2022-07-09 23:50:15 Making CallContext and InlineCodeBlock less special-case-y