On 06.12.23 21:02, Peter Eisentraut wrote:
I have been looking into what it would take to get rid of the
custom_read_write and custom_query_jumble for the RangeTblEntry node
type. This is one of the larger and more complex exceptions left.
(Similar considerations would also apply to the Constraint node type.)
In this updated patch set, I have also added the treatment of the
Constraint type. (I also noted that the manual read/write functions for
the Constraint type are out-of-sync again, so simplifying this would be
really helpful.) I have also added commit messages to each patch.
The way I have re-ordered the patch series now, I think patches 0001
through 0003 are candidates for inclusion after review, patch 0004 still
needs a bit more analysis and testing, as described therein.
From f0e896a201367a639be9d08d070066867c46d7cf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 11:23:39 +0100
Subject: [PATCH v2 1/4] Remove custom Constraint node read/write
implementations
This is part of an effort to reduce the number of special cases in the
automatically generated node support functions.
Allegedly, only certain fields of the Constraint node are valid based
on contype. But this has historically not been kept up to date in the
read/write functions. The Constraint node is only used for debugging
DDL statements, so there are no strong requirements for its output,
and there is no enforcement for its correctness. (There was no read
support before a6bc3301925.) Commits e7a552f303c and abf46ad9c7b are
examples of where omissions were fixed. And the current output is
again incorrect, because the recently added "inhcount" field is not
included.
This patch just removes the custom read/write implementations for the
Constraint node type. Now we just output all the fields, which is a
bit more than before, but at least we don't have to maintain these
functions anymore. Also, we lose the string representation of the
contype field, but for this marginal use case that seems tolerable.
This patch also changes the documentation of the Constraint struct to
put less emphasis on grouping fields by constraint type but rather
document for each field how it's used.
Discussion:
https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca...@eisentraut.org
---
src/backend/nodes/outfuncs.c | 126 -----------------------------
src/backend/nodes/readfuncs.c | 143 ---------------------------------
src/include/nodes/parsenodes.h | 38 +++------
3 files changed, 13 insertions(+), 294 deletions(-)
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 296ba845187..dee2b9e87b2 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -699,132 +699,6 @@ _outA_Const(StringInfo str, const A_Const *node)
WRITE_LOCATION_FIELD(location);
}
-static void
-_outConstraint(StringInfo str, const Constraint *node)
-{
- WRITE_NODE_TYPE("CONSTRAINT");
-
- WRITE_STRING_FIELD(conname);
- WRITE_BOOL_FIELD(deferrable);
- WRITE_BOOL_FIELD(initdeferred);
- WRITE_LOCATION_FIELD(location);
-
- appendStringInfoString(str, " :contype ");
- switch (node->contype)
- {
- case CONSTR_NULL:
- appendStringInfoString(str, "NULL");
- break;
-
- case CONSTR_NOTNULL:
- appendStringInfoString(str, "NOT_NULL");
- WRITE_NODE_FIELD(keys);
- WRITE_INT_FIELD(inhcount);
- WRITE_BOOL_FIELD(is_no_inherit);
- WRITE_BOOL_FIELD(skip_validation);
- WRITE_BOOL_FIELD(initially_valid);
- break;
-
- case CONSTR_DEFAULT:
- appendStringInfoString(str, "DEFAULT");
- WRITE_NODE_FIELD(raw_expr);
- WRITE_STRING_FIELD(cooked_expr);
- break;
-
- case CONSTR_IDENTITY:
- appendStringInfoString(str, "IDENTITY");
- WRITE_NODE_FIELD(options);
- WRITE_CHAR_FIELD(generated_when);
- break;
-
- case CONSTR_GENERATED:
- appendStringInfoString(str, "GENERATED");
- WRITE_NODE_FIELD(raw_expr);
- WRITE_STRING_FIELD(cooked_expr);
- WRITE_CHAR_FIELD(generated_when);
- break;
-
- case CONSTR_CHECK:
- appendStringInfoString(str, "CHECK");
- WRITE_BOOL_FIELD(is_no_inherit);
- WRITE_NODE_FIELD(raw_expr);
- WRITE_STRING_FIELD(cooked_expr);
- WRITE_BOOL_FIELD(skip_validation);
- WRITE_BOOL_FIELD(initially_valid);
- break;
-
- case CONSTR_PRIMARY:
- appendStringInfoString(str, "PRIMARY_KEY");
- WRITE_NODE_FIELD(keys);
- WRITE_NODE_FIELD(including);
- WRITE_NODE_FIELD(options);
- WRITE_STRING_FIELD(indexname);
- WRITE_STRING_FIELD(indexspace);
- WRITE_BOOL_FIELD(reset_default_tblspc);
- /* access_method and where_clause not currently used */
- break;
-
- case CONSTR_UNIQUE:
- appendStringInfoString(str, "UNIQUE");
- WRITE_BOOL_FIELD(nulls_not_distinct);
- WRITE_NODE_FIELD(keys);
- WRITE_NODE_FIELD(including);
- WRITE_NODE_FIELD(options);
- WRITE_STRING_FIELD(indexname);
- WRITE_STRING_FIELD(indexspace);
- WRITE_BOOL_FIELD(reset_default_tblspc);
- /* access_method and where_clause not currently used */
- break;
-
- case CONSTR_EXCLUSION:
- appendStringInfoString(str, "EXCLUSION");
- WRITE_NODE_FIELD(exclusions);
- WRITE_NODE_FIELD(including);
- WRITE_NODE_FIELD(options);
- WRITE_STRING_FIELD(indexname);
- WRITE_STRING_FIELD(indexspace);
- WRITE_BOOL_FIELD(reset_default_tblspc);
- WRITE_STRING_FIELD(access_method);
- WRITE_NODE_FIELD(where_clause);
- break;
-
- case CONSTR_FOREIGN:
- appendStringInfoString(str, "FOREIGN_KEY");
- WRITE_NODE_FIELD(pktable);
- WRITE_NODE_FIELD(fk_attrs);
- WRITE_NODE_FIELD(pk_attrs);
- WRITE_CHAR_FIELD(fk_matchtype);
- WRITE_CHAR_FIELD(fk_upd_action);
- WRITE_CHAR_FIELD(fk_del_action);
- WRITE_NODE_FIELD(fk_del_set_cols);
- WRITE_NODE_FIELD(old_conpfeqop);
- WRITE_OID_FIELD(old_pktable_oid);
- WRITE_BOOL_FIELD(skip_validation);
- WRITE_BOOL_FIELD(initially_valid);
- break;
-
- case CONSTR_ATTR_DEFERRABLE:
- appendStringInfoString(str, "ATTR_DEFERRABLE");
- break;
-
- case CONSTR_ATTR_NOT_DEFERRABLE:
- appendStringInfoString(str, "ATTR_NOT_DEFERRABLE");
- break;
-
- case CONSTR_ATTR_DEFERRED:
- appendStringInfoString(str, "ATTR_DEFERRED");
- break;
-
- case CONSTR_ATTR_IMMEDIATE:
- appendStringInfoString(str, "ATTR_IMMEDIATE");
- break;
-
- default:
- elog(ERROR, "unrecognized ConstrType: %d", (int)
node->contype);
- break;
- }
-}
-
/*
* outNode -
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 1624b345812..b1e2f2b440a 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -343,149 +343,6 @@ _readA_Const(void)
READ_DONE();
}
-/*
- * _readConstraint
- */
-static Constraint *
-_readConstraint(void)
-{
- READ_LOCALS(Constraint);
-
- READ_STRING_FIELD(conname);
- READ_BOOL_FIELD(deferrable);
- READ_BOOL_FIELD(initdeferred);
- READ_LOCATION_FIELD(location);
-
- token = pg_strtok(&length); /* skip :contype */
- token = pg_strtok(&length); /* get field value */
- if (length == 4 && strncmp(token, "NULL", 4) == 0)
- local_node->contype = CONSTR_NULL;
- else if (length == 8 && strncmp(token, "NOT_NULL", 8) == 0)
- local_node->contype = CONSTR_NOTNULL;
- else if (length == 7 && strncmp(token, "DEFAULT", 7) == 0)
- local_node->contype = CONSTR_DEFAULT;
- else if (length == 8 && strncmp(token, "IDENTITY", 8) == 0)
- local_node->contype = CONSTR_IDENTITY;
- else if (length == 9 && strncmp(token, "GENERATED", 9) == 0)
- local_node->contype = CONSTR_GENERATED;
- else if (length == 5 && strncmp(token, "CHECK", 5) == 0)
- local_node->contype = CONSTR_CHECK;
- else if (length == 11 && strncmp(token, "PRIMARY_KEY", 11) == 0)
- local_node->contype = CONSTR_PRIMARY;
- else if (length == 6 && strncmp(token, "UNIQUE", 6) == 0)
- local_node->contype = CONSTR_UNIQUE;
- else if (length == 9 && strncmp(token, "EXCLUSION", 9) == 0)
- local_node->contype = CONSTR_EXCLUSION;
- else if (length == 11 && strncmp(token, "FOREIGN_KEY", 11) == 0)
- local_node->contype = CONSTR_FOREIGN;
- else if (length == 15 && strncmp(token, "ATTR_DEFERRABLE", 15) == 0)
- local_node->contype = CONSTR_ATTR_DEFERRABLE;
- else if (length == 19 && strncmp(token, "ATTR_NOT_DEFERRABLE", 19) == 0)
- local_node->contype = CONSTR_ATTR_NOT_DEFERRABLE;
- else if (length == 13 && strncmp(token, "ATTR_DEFERRED", 13) == 0)
- local_node->contype = CONSTR_ATTR_DEFERRED;
- else if (length == 14 && strncmp(token, "ATTR_IMMEDIATE", 14) == 0)
- local_node->contype = CONSTR_ATTR_IMMEDIATE;
-
- switch (local_node->contype)
- {
- case CONSTR_NULL:
- /* no extra fields */
- break;
-
- case CONSTR_NOTNULL:
- READ_NODE_FIELD(keys);
- READ_INT_FIELD(inhcount);
- READ_BOOL_FIELD(is_no_inherit);
- READ_BOOL_FIELD(skip_validation);
- READ_BOOL_FIELD(initially_valid);
- break;
-
- case CONSTR_DEFAULT:
- READ_NODE_FIELD(raw_expr);
- READ_STRING_FIELD(cooked_expr);
- break;
-
- case CONSTR_IDENTITY:
- READ_NODE_FIELD(options);
- READ_CHAR_FIELD(generated_when);
- break;
-
- case CONSTR_GENERATED:
- READ_NODE_FIELD(raw_expr);
- READ_STRING_FIELD(cooked_expr);
- READ_CHAR_FIELD(generated_when);
- break;
-
- case CONSTR_CHECK:
- READ_BOOL_FIELD(is_no_inherit);
- READ_NODE_FIELD(raw_expr);
- READ_STRING_FIELD(cooked_expr);
- READ_BOOL_FIELD(skip_validation);
- READ_BOOL_FIELD(initially_valid);
- break;
-
- case CONSTR_PRIMARY:
- READ_NODE_FIELD(keys);
- READ_NODE_FIELD(including);
- READ_NODE_FIELD(options);
- READ_STRING_FIELD(indexname);
- READ_STRING_FIELD(indexspace);
- READ_BOOL_FIELD(reset_default_tblspc);
- /* access_method and where_clause not currently used */
- break;
-
- case CONSTR_UNIQUE:
- READ_BOOL_FIELD(nulls_not_distinct);
- READ_NODE_FIELD(keys);
- READ_NODE_FIELD(including);
- READ_NODE_FIELD(options);
- READ_STRING_FIELD(indexname);
- READ_STRING_FIELD(indexspace);
- READ_BOOL_FIELD(reset_default_tblspc);
- /* access_method and where_clause not currently used */
- break;
-
- case CONSTR_EXCLUSION:
- READ_NODE_FIELD(exclusions);
- READ_NODE_FIELD(including);
- READ_NODE_FIELD(options);
- READ_STRING_FIELD(indexname);
- READ_STRING_FIELD(indexspace);
- READ_BOOL_FIELD(reset_default_tblspc);
- READ_STRING_FIELD(access_method);
- READ_NODE_FIELD(where_clause);
- break;
-
- case CONSTR_FOREIGN:
- READ_NODE_FIELD(pktable);
- READ_NODE_FIELD(fk_attrs);
- READ_NODE_FIELD(pk_attrs);
- READ_CHAR_FIELD(fk_matchtype);
- READ_CHAR_FIELD(fk_upd_action);
- READ_CHAR_FIELD(fk_del_action);
- READ_NODE_FIELD(fk_del_set_cols);
- READ_NODE_FIELD(old_conpfeqop);
- READ_OID_FIELD(old_pktable_oid);
- READ_BOOL_FIELD(skip_validation);
- READ_BOOL_FIELD(initially_valid);
- break;
-
- case CONSTR_ATTR_DEFERRABLE:
- case CONSTR_ATTR_NOT_DEFERRABLE:
- case CONSTR_ATTR_DEFERRED:
- case CONSTR_ATTR_IMMEDIATE:
- /* no extra fields */
- break;
-
- default:
- elog(ERROR, "unrecognized ConstrType: %d", (int)
local_node->contype);
- break;
- }
-
- READ_DONE();
-}
-
static RangeTblEntry *
_readRangeTblEntry(void)
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b3181f34aee..648b6197502 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2566,43 +2566,33 @@ typedef enum ConstrType /* types of
constraints */
typedef struct Constraint
{
- pg_node_attr(custom_read_write)
-
NodeTag type;
ConstrType contype; /* see above */
-
- /* Fields used for most/all constraint types: */
char *conname; /* Constraint name, or NULL if unnamed
*/
bool deferrable; /* DEFERRABLE? */
bool initdeferred; /* INITIALLY DEFERRED? */
- int location; /* token location, or
-1 if unknown */
-
- /* Fields used for constraints with expressions (CHECK and DEFAULT): */
+ bool skip_validation; /* skip validation of existing
rows? */
+ bool initially_valid; /* mark the new constraint as
valid? */
bool is_no_inherit; /* is constraint non-inheritable? */
- Node *raw_expr; /* expr, as untransformed parse tree */
- char *cooked_expr; /* expr, as nodeToString representation
*/
+ Node *raw_expr; /* CHECK or DEFAULT expression, as
+ *
untransformed parse tree */
+ char *cooked_expr; /* CHECK or DEFAULT expression, as
+ * nodeToString
representation */
char generated_when; /* ALWAYS or BY DEFAULT */
-
- /* Fields used for "raw" NOT NULL constraints: */
- int inhcount; /* initial inheritance
count to apply */
-
- /* Fields used for unique constraints (UNIQUE and PRIMARY KEY): */
+ int inhcount; /* initial inheritance
count to apply, for
+ * "raw" NOT
NULL constraints */
bool nulls_not_distinct; /* null treatment for UNIQUE
constraints */
List *keys; /* String nodes naming
referenced key
- * column(s);
also used for NOT NULL */
+ * column(s);
for UNIQUE/PK/NOT NULL */
List *including; /* String nodes naming referenced nonkey
- * column(s) */
-
- /* Fields used for EXCLUSION constraints: */
- List *exclusions; /* list of (IndexElem, operator name)
pairs */
-
- /* Fields used for index constraints (UNIQUE, PRIMARY KEY, EXCLUSION):
*/
+ * column(s);
for UNIQUE/PK */
+ List *exclusions; /* list of (IndexElem, operator name)
pairs;
+ * for
exclusion constraints */
List *options; /* options from WITH clause */
char *indexname; /* existing index to use; otherwise
NULL */
char *indexspace; /* index tablespace; NULL for default */
bool reset_default_tblspc; /* reset default_tablespace
prior to
* creating the index */
- /* These could be, but currently are not, used for UNIQUE/PKEY: */
char *access_method; /* index access method; NULL for
default */
Node *where_clause; /* partial index predicate */
@@ -2618,9 +2608,7 @@ typedef struct Constraint
Oid old_pktable_oid; /*
pg_constraint.confrelid of my former
* self
*/
- /* Fields used for constraints that allow a NOT VALID specification */
- bool skip_validation; /* skip validation of existing
rows? */
- bool initially_valid; /* mark the new constraint as
valid? */
+ int location; /* token location, or
-1 if unknown */
} Constraint;
/* ----------------------
base-commit: 31acee4b66f9f88ad5c19c1276252688bdaa95c9
--
2.43.0
From 191aae34189594314042299f50309a1de3574be9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 11:23:39 +0100
Subject: [PATCH v2 2/4] Remove custom _jumbleRangeTblEntry()
This is part of an effort to reduce the number of special cases in the
automatically generated node support functions.
RangeTblEntry has a custom jumble function. This was probably just
carried over from the pg_stat_statements-specific code without any
detailed review. For example, the "inh" field is documented to be
valid in all RTEs, but it's only jumbled for RTE_RELATION. The
"lateral" field isn't looked at at all. I wouldn't be surprised if
there are more cases like this.
This patch removes _jumbleRangeTblEntry() and instead adds per-field
query_jumble_ignore annotations to approximately match the behavior of
the previous custom code. The pg_stat_statements test suite has some
coverage of this. It gets rid of switch on rtekind; this should be
technically correct, since we do the equal and copy functions like
this also. So for example the "inh" field is now considered in each
case. But it leaves "lateral" alone. Probably, several of these new
query_jumble_ignore should actually be dropped because the code was
wrong before.
Discussion:
https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca...@eisentraut.org
---
src/backend/nodes/queryjumblefuncs.c | 48 ----------------------------
src/include/nodes/parsenodes.h | 42 ++++++++++++------------
2 files changed, 21 insertions(+), 69 deletions(-)
diff --git a/src/backend/nodes/queryjumblefuncs.c
b/src/backend/nodes/queryjumblefuncs.c
index e489bfceb56..7accd7b6242 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -347,51 +347,3 @@ _jumbleA_Const(JumbleState *jstate, Node *node)
}
}
}
-
-static void
-_jumbleRangeTblEntry(JumbleState *jstate, Node *node)
-{
- RangeTblEntry *expr = (RangeTblEntry *) node;
-
- JUMBLE_FIELD(rtekind);
- switch (expr->rtekind)
- {
- case RTE_RELATION:
- JUMBLE_FIELD(relid);
- JUMBLE_NODE(tablesample);
- JUMBLE_FIELD(inh);
- break;
- case RTE_SUBQUERY:
- JUMBLE_NODE(subquery);
- break;
- case RTE_JOIN:
- JUMBLE_FIELD(jointype);
- break;
- case RTE_FUNCTION:
- JUMBLE_NODE(functions);
- break;
- case RTE_TABLEFUNC:
- JUMBLE_NODE(tablefunc);
- break;
- case RTE_VALUES:
- JUMBLE_NODE(values_lists);
- break;
- case RTE_CTE:
-
- /*
- * Depending on the CTE name here isn't ideal, but it's
the only
- * info we have to identify the referenced WITH item.
- */
- JUMBLE_STRING(ctename);
- JUMBLE_FIELD(ctelevelsup);
- break;
- case RTE_NAMEDTUPLESTORE:
- JUMBLE_STRING(enrname);
- break;
- case RTE_RESULT:
- break;
- default:
- elog(ERROR, "unrecognized RTE kind: %d", (int)
expr->rtekind);
- break;
- }
-}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 648b6197502..d377f16a72b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1018,7 +1018,7 @@ typedef enum RTEKind
typedef struct RangeTblEntry
{
- pg_node_attr(custom_read_write, custom_query_jumble)
+ pg_node_attr(custom_read_write)
NodeTag type;
@@ -1062,16 +1062,16 @@ typedef struct RangeTblEntry
* tables to be invalidated if the underlying table is altered.
*/
Oid relid; /* OID of the relation
*/
- char relkind; /* relation kind (see
pg_class.relkind) */
- int rellockmode; /* lock level that query
requires on the rel */
+ char relkind pg_node_attr(query_jumble_ignore);
/* relation kind (see pg_class.relkind) */
+ int rellockmode pg_node_attr(query_jumble_ignore);
/* lock level that query requires on the rel */
struct TableSampleClause *tablesample; /* sampling info, or NULL */
- Index perminfoindex;
+ Index perminfoindex pg_node_attr(query_jumble_ignore);
/*
* Fields valid for a subquery RTE (else NULL):
*/
Query *subquery; /* the sub-query */
- bool security_barrier; /* is from security_barrier
view? */
+ bool security_barrier pg_node_attr(query_jumble_ignore);
/* is from security_barrier view? */
/*
* Fields valid for a join RTE (else NULL/zero):
@@ -1117,17 +1117,17 @@ typedef struct RangeTblEntry
* joinleftcols/joinrighttcols.
*/
JoinType jointype; /* type of join */
- int joinmergedcols; /* number of merged (JOIN
USING) columns */
- List *joinaliasvars; /* list of alias-var expansions */
- List *joinleftcols; /* left-side input column numbers */
- List *joinrightcols; /* right-side input column numbers */
+ int joinmergedcols
pg_node_attr(query_jumble_ignore); /* number of merged (JOIN USING) columns */
+ List *joinaliasvars pg_node_attr(query_jumble_ignore); /* list
of alias-var expansions */
+ List *joinleftcols pg_node_attr(query_jumble_ignore); /*
left-side input column numbers */
+ List *joinrightcols pg_node_attr(query_jumble_ignore); /*
right-side input column numbers */
/*
* join_using_alias is an alias clause attached directly to JOIN/USING.
It
* is different from the alias field (below) in that it does not hide
the
* range variables of the tables being joined.
*/
- Alias *join_using_alias;
+ Alias *join_using_alias pg_node_attr(query_jumble_ignore);
/*
* Fields valid for a function RTE (else NIL/zero):
@@ -1138,7 +1138,7 @@ typedef struct RangeTblEntry
* expandRTE().
*/
List *functions; /* list of RangeTblFunction nodes */
- bool funcordinality; /* is this called WITH ORDINALITY? */
+ bool funcordinality pg_node_attr(query_jumble_ignore); /* is
this called WITH ORDINALITY? */
/*
* Fields valid for a TableFunc RTE (else NULL):
@@ -1155,7 +1155,7 @@ typedef struct RangeTblEntry
*/
char *ctename; /* name of the WITH list item */
Index ctelevelsup; /* number of query levels up */
- bool self_reference; /* is this a recursive self-reference?
*/
+ bool self_reference pg_node_attr(query_jumble_ignore); /* is
this a recursive self-reference? */
/*
* Fields valid for CTE, VALUES, ENR, and TableFunc RTEs (else NIL):
@@ -1175,25 +1175,25 @@ typedef struct RangeTblEntry
* all three lists (as well as an empty-string entry in eref). Testing
* for zero coltype is the standard way to detect a dropped column.
*/
- List *coltypes; /* OID list of column type OIDs */
- List *coltypmods; /* integer list of column typmods */
- List *colcollations; /* OID list of column collation OIDs */
+ List *coltypes pg_node_attr(query_jumble_ignore); /* OID
list of column type OIDs */
+ List *coltypmods pg_node_attr(query_jumble_ignore);
/* integer list of column typmods */
+ List *colcollations pg_node_attr(query_jumble_ignore); /* OID
list of column collation OIDs */
/*
* Fields valid for ENR RTEs (else NULL/zero):
*/
char *enrname; /* name of ephemeral named relation */
- Cardinality enrtuples; /* estimated or actual from caller */
+ Cardinality enrtuples pg_node_attr(query_jumble_ignore);
/* estimated or actual from caller */
/*
* Fields valid in all RTEs:
*/
- Alias *alias; /* user-written alias clause,
if any */
- Alias *eref; /* expanded reference names */
- bool lateral; /* subquery, function, or
values is LATERAL? */
+ Alias *alias pg_node_attr(query_jumble_ignore);
/* user-written alias clause, if any */
+ Alias *eref pg_node_attr(query_jumble_ignore);
/* expanded reference names */
+ bool lateral pg_node_attr(query_jumble_ignore);
/* subquery, function, or values is LATERAL? */
bool inh; /* inheritance requested? */
- bool inFromCl; /* present in FROM clause? */
- List *securityQuals; /* security barrier quals to apply, if
any */
+ bool inFromCl pg_node_attr(query_jumble_ignore);
/* present in FROM clause? */
+ List *securityQuals pg_node_attr(query_jumble_ignore); /*
security barrier quals to apply, if any */
} RangeTblEntry;
/*
--
2.43.0
From df2898235350aafe82fb948b73e12a976e71237a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 11:23:39 +0100
Subject: [PATCH v2 3/4] Simplify range_table_mutator_impl()
This is part of an effort to reduce the number of special cases in the
node support functions.
Allegedly, only certain fields of RangeTblEntry are valid based on
rtekind. But exactly which ones seems to be documented and handled
inconsistently. It seems that over time, new RTE kinds have
"borrowed" fields that notionally belong to other RTE kinds, which is
technically not a problem but creates a bit of a mess when trying to
understand all this.
This patch removes the switch on rtekind from
range_table_mutator_impl(). This should be fine because all the
subroutines can handle unset/NULL fields. And it removes one more
place that needs to track knowledge about which fields are valid when.
Discussion:
https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca...@eisentraut.org
---
src/backend/nodes/nodeFuncs.c | 61 +++++++++++++----------------------
1 file changed, 23 insertions(+), 38 deletions(-)
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index e1a5bc7e95d..fe87238f195 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3694,47 +3694,32 @@ range_table_mutator_impl(List *rtable,
RangeTblEntry *newrte;
FLATCOPY(newrte, rte, RangeTblEntry);
- switch (rte->rtekind)
+
+ MUTATE(newrte->tablesample, rte->tablesample, TableSampleClause
*);
+
+ if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
+ MUTATE(newrte->subquery, rte->subquery, Query *);
+ else
{
- case RTE_RELATION:
- MUTATE(newrte->tablesample, rte->tablesample,
- TableSampleClause *);
- /* we don't bother to copy eref, aliases, etc;
OK? */
- break;
- case RTE_SUBQUERY:
- if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
- MUTATE(newrte->subquery, rte->subquery,
Query *);
- else
- {
- /* else, copy RT subqueries as-is */
- newrte->subquery =
copyObject(rte->subquery);
- }
- break;
- case RTE_JOIN:
- if (!(flags & QTW_IGNORE_JOINALIASES))
- MUTATE(newrte->joinaliasvars,
rte->joinaliasvars, List *);
- else
- {
- /* else, copy join aliases as-is */
- newrte->joinaliasvars =
copyObject(rte->joinaliasvars);
- }
- break;
- case RTE_FUNCTION:
- MUTATE(newrte->functions, rte->functions, List
*);
- break;
- case RTE_TABLEFUNC:
- MUTATE(newrte->tablefunc, rte->tablefunc,
TableFunc *);
- break;
- case RTE_VALUES:
- MUTATE(newrte->values_lists, rte->values_lists,
List *);
- break;
- case RTE_CTE:
- case RTE_NAMEDTUPLESTORE:
- case RTE_RESULT:
- /* nothing to do */
- break;
+ /* else, copy RT subqueries as-is */
+ newrte->subquery = copyObject(rte->subquery);
+ }
+
+ if (!(flags & QTW_IGNORE_JOINALIASES))
+ MUTATE(newrte->joinaliasvars, rte->joinaliasvars, List
*);
+ else
+ {
+ /* else, copy join aliases as-is */
+ newrte->joinaliasvars = copyObject(rte->joinaliasvars);
}
+
+ MUTATE(newrte->functions, rte->functions, List *);
+ MUTATE(newrte->tablefunc, rte->tablefunc, TableFunc *);
+ MUTATE(newrte->values_lists, rte->values_lists, List *);
MUTATE(newrte->securityQuals, rte->securityQuals, List *);
+
+ /* we don't bother to copy eref, aliases, etc; OK? */
+
newrt = lappend(newrt, newrte);
}
return newrt;
--
2.43.0
From 3f8c3217e529e2b5a07e9ea226fc71eb33e485ad Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Mon, 15 Jan 2024 11:23:39 +0100
Subject: [PATCH v2 4/4] WIP: Remove custom RangeTblEntry node read/write
implementations
This is part of an effort to reduce the number of special cases in the
automatically generated node support functions.
Allegedly, only certain fields of RangeTblEntry are valid based on
rtekind. But exactly which ones seems to be documented and handled
inconsistently. It seems that over time, new RTE kinds have
"borrowed" fields that notionally belong to other RTE kinds, which is
technically not a problem but creates a bit of a mess when trying to
understand all this.
This patch removes the custom read/write functions for RangeTblEntry.
Those functions wanted to have a few fields at the front to make the
dump more legible; this is done now by moving the fields up in the
actual struct.
Discussion:
https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca...@eisentraut.org
TODO: clean up the documentation RangeTblEntry node
TODO: check how much this bloats stored rules
TODO: catversion
---
src/backend/nodes/outfuncs.c | 80 -----------------------------
src/backend/nodes/readfuncs.c | 92 ----------------------------------
src/include/nodes/parsenodes.h | 12 +++--
3 files changed, 8 insertions(+), 176 deletions(-)
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index dee2b9e87b2..26cfd60aed9 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -489,86 +489,6 @@ _outExtensibleNode(StringInfo str, const ExtensibleNode
*node)
methods->nodeOut(str, node);
}
-static void
-_outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
-{
- WRITE_NODE_TYPE("RANGETBLENTRY");
-
- /* put alias + eref first to make dump more legible */
- WRITE_NODE_FIELD(alias);
- WRITE_NODE_FIELD(eref);
- WRITE_ENUM_FIELD(rtekind, RTEKind);
-
- switch (node->rtekind)
- {
- case RTE_RELATION:
- WRITE_OID_FIELD(relid);
- WRITE_CHAR_FIELD(relkind);
- WRITE_INT_FIELD(rellockmode);
- WRITE_NODE_FIELD(tablesample);
- WRITE_UINT_FIELD(perminfoindex);
- break;
- case RTE_SUBQUERY:
- WRITE_NODE_FIELD(subquery);
- WRITE_BOOL_FIELD(security_barrier);
- /* we re-use these RELATION fields, too: */
- WRITE_OID_FIELD(relid);
- WRITE_CHAR_FIELD(relkind);
- WRITE_INT_FIELD(rellockmode);
- WRITE_UINT_FIELD(perminfoindex);
- break;
- case RTE_JOIN:
- WRITE_ENUM_FIELD(jointype, JoinType);
- WRITE_INT_FIELD(joinmergedcols);
- WRITE_NODE_FIELD(joinaliasvars);
- WRITE_NODE_FIELD(joinleftcols);
- WRITE_NODE_FIELD(joinrightcols);
- WRITE_NODE_FIELD(join_using_alias);
- break;
- case RTE_FUNCTION:
- WRITE_NODE_FIELD(functions);
- WRITE_BOOL_FIELD(funcordinality);
- break;
- case RTE_TABLEFUNC:
- WRITE_NODE_FIELD(tablefunc);
- break;
- case RTE_VALUES:
- WRITE_NODE_FIELD(values_lists);
- WRITE_NODE_FIELD(coltypes);
- WRITE_NODE_FIELD(coltypmods);
- WRITE_NODE_FIELD(colcollations);
- break;
- case RTE_CTE:
- WRITE_STRING_FIELD(ctename);
- WRITE_UINT_FIELD(ctelevelsup);
- WRITE_BOOL_FIELD(self_reference);
- WRITE_NODE_FIELD(coltypes);
- WRITE_NODE_FIELD(coltypmods);
- WRITE_NODE_FIELD(colcollations);
- break;
- case RTE_NAMEDTUPLESTORE:
- WRITE_STRING_FIELD(enrname);
- WRITE_FLOAT_FIELD(enrtuples);
- WRITE_NODE_FIELD(coltypes);
- WRITE_NODE_FIELD(coltypmods);
- WRITE_NODE_FIELD(colcollations);
- /* we re-use these RELATION fields, too: */
- WRITE_OID_FIELD(relid);
- break;
- case RTE_RESULT:
- /* no extra fields */
- break;
- default:
- elog(ERROR, "unrecognized RTE kind: %d", (int)
node->rtekind);
- break;
- }
-
- WRITE_BOOL_FIELD(lateral);
- WRITE_BOOL_FIELD(inh);
- WRITE_BOOL_FIELD(inFromCl);
- WRITE_NODE_FIELD(securityQuals);
-}
-
static void
_outA_Expr(StringInfo str, const A_Expr *node)
{
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index b1e2f2b440a..54d3eecc1bc 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -343,98 +343,6 @@ _readA_Const(void)
READ_DONE();
}
-static RangeTblEntry *
-_readRangeTblEntry(void)
-{
- READ_LOCALS(RangeTblEntry);
-
- /* put alias + eref first to make dump more legible */
- READ_NODE_FIELD(alias);
- READ_NODE_FIELD(eref);
- READ_ENUM_FIELD(rtekind, RTEKind);
-
- switch (local_node->rtekind)
- {
- case RTE_RELATION:
- READ_OID_FIELD(relid);
- READ_CHAR_FIELD(relkind);
- READ_INT_FIELD(rellockmode);
- READ_NODE_FIELD(tablesample);
- READ_UINT_FIELD(perminfoindex);
- break;
- case RTE_SUBQUERY:
- READ_NODE_FIELD(subquery);
- READ_BOOL_FIELD(security_barrier);
- /* we re-use these RELATION fields, too: */
- READ_OID_FIELD(relid);
- READ_CHAR_FIELD(relkind);
- READ_INT_FIELD(rellockmode);
- READ_UINT_FIELD(perminfoindex);
- break;
- case RTE_JOIN:
- READ_ENUM_FIELD(jointype, JoinType);
- READ_INT_FIELD(joinmergedcols);
- READ_NODE_FIELD(joinaliasvars);
- READ_NODE_FIELD(joinleftcols);
- READ_NODE_FIELD(joinrightcols);
- READ_NODE_FIELD(join_using_alias);
- break;
- case RTE_FUNCTION:
- READ_NODE_FIELD(functions);
- READ_BOOL_FIELD(funcordinality);
- break;
- case RTE_TABLEFUNC:
- READ_NODE_FIELD(tablefunc);
- /* The RTE must have a copy of the column type info, if
any */
- if (local_node->tablefunc)
- {
- TableFunc *tf = local_node->tablefunc;
-
- local_node->coltypes = tf->coltypes;
- local_node->coltypmods = tf->coltypmods;
- local_node->colcollations = tf->colcollations;
- }
- break;
- case RTE_VALUES:
- READ_NODE_FIELD(values_lists);
- READ_NODE_FIELD(coltypes);
- READ_NODE_FIELD(coltypmods);
- READ_NODE_FIELD(colcollations);
- break;
- case RTE_CTE:
- READ_STRING_FIELD(ctename);
- READ_UINT_FIELD(ctelevelsup);
- READ_BOOL_FIELD(self_reference);
- READ_NODE_FIELD(coltypes);
- READ_NODE_FIELD(coltypmods);
- READ_NODE_FIELD(colcollations);
- break;
- case RTE_NAMEDTUPLESTORE:
- READ_STRING_FIELD(enrname);
- READ_FLOAT_FIELD(enrtuples);
- READ_NODE_FIELD(coltypes);
- READ_NODE_FIELD(coltypmods);
- READ_NODE_FIELD(colcollations);
- /* we re-use these RELATION fields, too: */
- READ_OID_FIELD(relid);
- break;
- case RTE_RESULT:
- /* no extra fields */
- break;
- default:
- elog(ERROR, "unrecognized RTE kind: %d",
- (int) local_node->rtekind);
- break;
- }
-
- READ_BOOL_FIELD(lateral);
- READ_BOOL_FIELD(inh);
- READ_BOOL_FIELD(inFromCl);
- READ_NODE_FIELD(securityQuals);
-
- READ_DONE();
-}
-
static A_Expr *
_readA_Expr(void)
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index d377f16a72b..8042c5d91ed 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1018,8 +1018,6 @@ typedef enum RTEKind
typedef struct RangeTblEntry
{
- pg_node_attr(custom_read_write)
-
NodeTag type;
RTEKind rtekind; /* see above */
@@ -1030,6 +1028,14 @@ typedef struct RangeTblEntry
* code that is being actively worked on. FIXME someday.
*/
+ /*
+ * Fields valid in all RTEs:
+ *
+ * put alias + eref first to make dump more legible
+ */
+ Alias *alias pg_node_attr(query_jumble_ignore);
/* user-written alias clause, if any */
+ Alias *eref pg_node_attr(query_jumble_ignore);
/* expanded reference names */
+
/*
* Fields valid for a plain relation RTE (else zero):
*
@@ -1188,8 +1194,6 @@ typedef struct RangeTblEntry
/*
* Fields valid in all RTEs:
*/
- Alias *alias pg_node_attr(query_jumble_ignore);
/* user-written alias clause, if any */
- Alias *eref pg_node_attr(query_jumble_ignore);
/* expanded reference names */
bool lateral pg_node_attr(query_jumble_ignore);
/* subquery, function, or values is LATERAL? */
bool inh; /* inheritance requested? */
bool inFromCl pg_node_attr(query_jumble_ignore);
/* present in FROM clause? */
--
2.43.0
