From 134e562354a91630dd8dbc10c5b334cd05e028f5 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sun, 4 Feb 2024 19:13:20 +0100 Subject: [PATCH] Improve pg_trigger.tgargs representation Before, the trigger argument list was encoded into a bytea column. Replace this with a list of nodes in a pg_node_tree. This makes the internal code much simpler and also allows clients to decode the column more easily. Clients shipped with PostgreSQL (such as psql and pg_dump) use pg_get_triggerdef() and don't access the field directly, so they need to no changes. TODO: catversion --- doc/src/sgml/catalogs.sgml | 14 +--- src/backend/catalog/information_schema.sql | 3 +- src/backend/commands/tablecmds.c | 27 +++----- src/backend/commands/trigger.c | 76 +++++----------------- src/backend/utils/adt/ruleutils.c | 31 +++++---- src/include/catalog/pg_trigger.h | 3 +- 6 files changed, 47 insertions(+), 107 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 880f717b103..9f319039353 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -8490,15 +8490,6 @@ <structname>pg_trigger</structname> Columns - - - tgnargs int2 - - - Number of argument strings passed to trigger function - - - tgattr int2vector @@ -8512,10 +8503,11 @@ <structname>pg_trigger</structname> Columns - tgargs bytea + tgargs pg_node_tree - Argument strings to pass to trigger, each NULL-terminated + List of argument strings to pass to trigger (in + nodeToString() representation) diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql index c402ae72741..866015faedc 100644 --- a/src/backend/catalog/information_schema.sql +++ b/src/backend/catalog/information_schema.sql @@ -2138,8 +2138,7 @@ CREATE VIEW triggers AS ELSE null END AS character_data) AS action_condition, CAST( - substring(pg_get_triggerdef(t.oid) from - position('EXECUTE FUNCTION' in substring(pg_get_triggerdef(t.oid) from 48)) + 47) + 'EXECUTE FUNCTION ' || t.tgfoid::regprocedure || '(' || pg_get_expr(t.tgargs, 0) || ')' AS character_data) AS action_statement, CAST( -- hard-wired reference to TRIGGER_TYPE_ROW diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9f51696740e..19a84f76008 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -19423,25 +19423,16 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) } } - /* Reconstruct trigger arguments list. */ - if (trigForm->tgnargs > 0) - { - char *p; - - value = heap_getattr(tuple, Anum_pg_trigger_tgargs, - RelationGetDescr(pg_trigger), &isnull); - if (isnull) - elog(ERROR, "tgargs is null for trigger \"%s\" in partition \"%s\"", - NameStr(trigForm->tgname), RelationGetRelationName(partition)); - - p = (char *) VARDATA_ANY(DatumGetByteaPP(value)); + /* + * Reconstruct trigger arguments list. + */ + value = heap_getattr(tuple, Anum_pg_trigger_tgargs, + RelationGetDescr(pg_trigger), &isnull); + if (isnull) + elog(ERROR, "tgargs is null for trigger \"%s\" in partition \"%s\"", + NameStr(trigForm->tgname), RelationGetRelationName(partition)); - for (int i = 0; i < trigForm->tgnargs; i++) - { - trigargs = lappend(trigargs, makeString(pstrdup(p))); - p += strlen(p) + 1; - } - } + trigargs = stringToNode(TextDatumGetCString(value)); trigStmt = makeNode(CreateTrigStmt); trigStmt->replace = false; diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index c344ff09442..f7f6650d881 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -881,50 +881,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, values[Anum_pg_trigger_tgconstraint - 1] = ObjectIdGetDatum(constraintOid); values[Anum_pg_trigger_tgdeferrable - 1] = BoolGetDatum(stmt->deferrable); values[Anum_pg_trigger_tginitdeferred - 1] = BoolGetDatum(stmt->initdeferred); - - if (stmt->args) - { - ListCell *le; - char *args; - int16 nargs = list_length(stmt->args); - int len = 0; - - foreach(le, stmt->args) - { - char *ar = strVal(lfirst(le)); - - len += strlen(ar) + 4; - for (; *ar; ar++) - { - if (*ar == '\\') - len++; - } - } - args = (char *) palloc(len + 1); - args[0] = '\0'; - foreach(le, stmt->args) - { - char *s = strVal(lfirst(le)); - char *d = args + strlen(args); - - while (*s) - { - if (*s == '\\') - *d++ = '\\'; - *d++ = *s++; - } - strcpy(d, "\\000"); - } - values[Anum_pg_trigger_tgnargs - 1] = Int16GetDatum(nargs); - values[Anum_pg_trigger_tgargs - 1] = DirectFunctionCall1(byteain, - CStringGetDatum(args)); - } - else - { - values[Anum_pg_trigger_tgnargs - 1] = Int16GetDatum(0); - values[Anum_pg_trigger_tgargs - 1] = DirectFunctionCall1(byteain, - CStringGetDatum("")); - } + values[Anum_pg_trigger_tgargs - 1] = CStringGetTextDatum(nodeToString(stmt->args)); /* build column number array if it's a column-specific trigger */ ncolumns = list_length(stmt->columns); @@ -1902,6 +1859,7 @@ RelationBuildTriggers(Relation relation) Trigger *build; Datum datum; bool isnull; + List *argslist; if (numtrigs >= maxtrigs) { @@ -1923,7 +1881,6 @@ RelationBuildTriggers(Relation relation) build->tgconstraint = pg_trigger->tgconstraint; build->tgdeferrable = pg_trigger->tgdeferrable; build->tginitdeferred = pg_trigger->tginitdeferred; - build->tgnargs = pg_trigger->tgnargs; /* tgattr is first var-width field, so OK to access directly */ build->tgnattr = pg_trigger->tgattr.dim1; if (build->tgnattr > 0) @@ -1934,24 +1891,23 @@ RelationBuildTriggers(Relation relation) } else build->tgattr = NULL; + + datum = fastgetattr(htup, Anum_pg_trigger_tgargs, + tgrel->rd_att, &isnull); + if (isnull) + elog(ERROR, "tgargs is null in trigger for relation \"%s\"", + RelationGetRelationName(relation)); + + argslist = castNode(List, stringToNode(TextDatumGetCString(datum))); + build->tgnargs = list_length(argslist); if (build->tgnargs > 0) { - bytea *val; - char *p; - - val = DatumGetByteaPP(fastgetattr(htup, - Anum_pg_trigger_tgargs, - tgrel->rd_att, &isnull)); - if (isnull) - elog(ERROR, "tgargs is null in trigger for relation \"%s\"", - RelationGetRelationName(relation)); - p = (char *) VARDATA_ANY(val); + ListCell *lc; + build->tgargs = (char **) palloc(build->tgnargs * sizeof(char *)); - for (i = 0; i < build->tgnargs; i++) - { - build->tgargs[i] = pstrdup(p); - p += strlen(p) + 1; - } + i = 0; + foreach(lc, argslist) + build->tgargs[i++] = pstrdup(strVal(lfirst(lc))); } else build->tgargs = NULL; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index b625f471a84..b97604e98ad 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1100,26 +1100,25 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) NIL, NULL, false, NULL, EXPR_KIND_NONE)); - if (trigrec->tgnargs > 0) { - char *p; - int i; + deparse_context context; value = fastgetattr(ht_trig, Anum_pg_trigger_tgargs, tgrel->rd_att, &isnull); if (isnull) elog(ERROR, "tgargs is null for trigger %u", trigid); - p = (char *) VARDATA_ANY(DatumGetByteaPP(value)); - for (i = 0; i < trigrec->tgnargs; i++) - { - if (i > 0) - appendStringInfoString(&buf, ", "); - simple_quote_literal(&buf, p); - /* advance p to next string embedded in tgargs */ - while (*p) - p++; - p++; - } + + context.buf = &buf; + context.namespaces = NIL; + context.windowClause = NIL; + context.windowTList = NIL; + context.varprefix = true; + context.prettyFlags = pretty ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) : PRETTYFLAG_INDENT; + context.wrapColumn = WRAP_COLUMN_DEFAULT; + context.indentLevel = PRETTYINDENT_STD; + context.special_exprkind = EXPR_KIND_NONE; + + get_rule_expr(stringToNode(TextDatumGetCString(value)), &context, false); } /* We deliberately do not put semi-colon at end */ @@ -9811,6 +9810,10 @@ get_rule_expr(Node *node, deparse_context *context, } break; + case T_String: + simple_quote_literal(buf, strVal(node)); + break; + case T_TableFunc: get_tablefunc((TableFunc *) node, context, showimplicit); break; diff --git a/src/include/catalog/pg_trigger.h b/src/include/catalog/pg_trigger.h index 7fdff161184..5bbf7294770 100644 --- a/src/include/catalog/pg_trigger.h +++ b/src/include/catalog/pg_trigger.h @@ -55,7 +55,6 @@ CATALOG(pg_trigger,2620,TriggerRelationId) * if any */ bool tgdeferrable; /* constraint trigger is deferrable */ bool tginitdeferred; /* constraint trigger is deferred initially */ - int16 tgnargs; /* # of extra arguments in tgargs */ /* * Variable-length fields start here, but we allow direct access to @@ -65,7 +64,7 @@ CATALOG(pg_trigger,2620,TriggerRelationId) * on columns */ #ifdef CATALOG_VARLEN - bytea tgargs BKI_FORCE_NOT_NULL; /* first\000second\000tgnargs\000 */ + pg_node_tree tgargs BKI_FORCE_NOT_NULL; /* list of arguments */ pg_node_tree tgqual; /* WHEN expression, or NULL if none */ NameData tgoldtable; /* old transition table, or NULL if none */ NameData tgnewtable; /* new transition table, or NULL if none */ base-commit: 774bcffe4a9853a24e61d758637c0aad2871f1fb -- 2.43.0