sql: make TG_ARGV use 0-based indexing for PostgreSQL compatibility

Previously, TG_ARGV used 1-based indexing (standard SQL array behavior),
which was incompatible with PostgreSQL's 0-based indexing for TG_ARGV.
A session setting `allow_create_trigger_function_with_argv_references`
was required to use TG_ARGV at all, since it was blocked by default due
to this incompatibility.

This change makes TG_ARGV use 0-based indexing to match PostgreSQL
behavior. To implement this, we add a `zeroIndexed` field to the DArray
struct that controls the FirstIndex() return value. When constructing
TG_ARGV for trigger functions, we now call SetZeroIndexed() to mark the
array as 0-indexed.

The `allow_create_trigger_function_with_argv_references` session setting
is no longer needed and has been converted to a backwards-compatibility
no-op variable.

Fixes: #135311

Release note (backward-incompatible change): The TG_ARGV trigger
function parameter now uses 0-based indexing to match PostgreSQL
behavior. Previously, TG_ARGV[1] returned the first argument; now
TG_ARGV[0] returns the first argument and TG_ARGV[1] returns the second
argument. Additionally, usage of TG_ARGV no longer requires setting the
`allow_create_trigger_function_with_argv_references` session variable.

Author: rafiss

pkg/ccl/logictestccl/testdata/logic_test/triggers

@@ -4213,8 +4213,7 @@ CREATE FUNCTION g() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
     RAISE NOTICE 'TG_TABLE_NAME: %', TG_TABLE_NAME;
     RAISE NOTICE 'TG_TABLE_SCHEMA: %', TG_TABLE_SCHEMA;
     RAISE NOTICE 'TG_NARGS: %', TG_NARGS;
-    -- TODO(#135311): uncomment this when we support TG_ARGV.
-    --RAISE NOTICE 'TG_ARGV: %s', TG_ARGV;
+    RAISE NOTICE 'TG_ARGV: %', TG_ARGV;
     RETURN NEW;
   END
 $$;
@@ -4239,6 +4238,7 @@ NOTICE: TG_RELNAME: xy
 NOTICE: TG_TABLE_NAME: xy
 NOTICE: TG_TABLE_SCHEMA: public
 NOTICE: TG_NARGS: 0
+NOTICE: TG_ARGV: {}
 NOTICE: NEW: (1,2)
 NOTICE: OLD: <NULL>
 NOTICE: TG_NAME: bar
@@ -4250,6 +4250,7 @@ NOTICE: TG_RELNAME: xy
 NOTICE: TG_TABLE_NAME: xy
 NOTICE: TG_TABLE_SCHEMA: public
 NOTICE: TG_NARGS: 0
+NOTICE: TG_ARGV: {}
 
 query T noticetrace
 UPDATE xy SET y = 3 WHERE x = 1;
@@ -4265,6 +4266,7 @@ NOTICE: TG_RELNAME: xy
 NOTICE: TG_TABLE_NAME: xy
 NOTICE: TG_TABLE_SCHEMA: public
 NOTICE: TG_NARGS: 0
+NOTICE: TG_ARGV: {}
 NOTICE: NEW: (1,3)
 NOTICE: OLD: (1,2)
 NOTICE: TG_NAME: bar
@@ -4276,6 +4278,7 @@ NOTICE: TG_RELNAME: xy
 NOTICE: TG_TABLE_NAME: xy
 NOTICE: TG_TABLE_SCHEMA: public
 NOTICE: TG_NARGS: 0
+NOTICE: TG_ARGV: {}
 
 statement ok
 DROP TRIGGER foo ON xy;
@@ -4795,8 +4798,7 @@ DROP FUNCTION f4();
 
 subtest end
 
-# Until #135311 can be fixed, disallow TG_ARGV references by default. There is
-# a setting to allow them if necessary.
+# Regression test for #135311: TG_ARGV should be 0-indexed like PostgreSQL.
 subtest regression_135311
 
 statement ok
@@ -4806,43 +4808,95 @@ statement ok
 CREATE FUNCTION g() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
   BEGIN
     RAISE NOTICE 'g()';
+    RAISE NOTICE 'TG_ARGV[0]: %', TG_ARGV[0];
     RAISE NOTICE 'TG_ARGV[1]: %', TG_ARGV[1];
     RETURN NEW;
   END
 $$;
 
-statement error pgcode 0A000 pq: unimplemented: referencing the TG_ARGV trigger function parameter is not yet supported
-CREATE TRIGGER foo BEFORE INSERT ON xy FOR EACH ROW EXECUTE FUNCTION g('foo', 'bar');
+statement ok
+CREATE TRIGGER foo BEFORE INSERT ON xy FOR EACH ROW EXECUTE FUNCTION g('first', 'second');
+
+query T noticetrace
+INSERT INTO xy VALUES (1, 2);
+----
+NOTICE: g()
+NOTICE: TG_ARGV[0]: first
+NOTICE: TG_ARGV[1]: second
 
 statement ok
-SET allow_create_trigger_function_with_argv_references = true;
+DROP TRIGGER foo ON xy;
 
 statement ok
-CREATE TRIGGER foo BEFORE INSERT ON xy FOR EACH ROW EXECUTE FUNCTION g('foo', 'bar');
+DROP FUNCTION g;
+
+# Test assigning TG_ARGV to a TEXT[] variable, then doing a zero-indexed read.
+# Note: This matches Postgres behavior, and keeps the array as zero-indexed.
+statement ok
+CREATE FUNCTION g() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
+  DECLARE
+    args TEXT[];
+  BEGIN
+    args := TG_ARGV;
+    RAISE NOTICE 'args[0]: %', args[0];
+    RAISE NOTICE 'args[1]: %', args[1];
+    RETURN NEW;
+  END
+$$;
+
+statement ok
+CREATE TRIGGER foo BEFORE INSERT ON xy FOR EACH ROW EXECUTE FUNCTION g('arg_one', 'arg_two');
 
 query T noticetrace
-INSERT INTO xy VALUES (1, 2);
+INSERT INTO xy VALUES (100, 200);
 ----
-NOTICE: g()
-NOTICE: TG_ARGV[1]: foo
+NOTICE: args[0]: arg_one
+NOTICE: args[1]: arg_two
 
 statement ok
-RESET allow_create_trigger_function_with_argv_references;
+DROP TRIGGER foo ON xy;
+
+statement ok
+DROP FUNCTION g;
+
+# Test a helper function that takes a TEXT[] argument and does a zero-indexed
+# read on it.
+# Note: In PostgreSQL, when TG_ARGV is passed to a function, it retains its
+# zero-indexed behavior.
+statement ok
+CREATE FUNCTION read_args(arr TEXT[]) RETURNS TEXT LANGUAGE PLpgSQL AS $$
+  BEGIN
+    RETURN 'arr[0]=' || COALESCE(arr[0], 'NULL') || ', arr[1]=' || COALESCE(arr[1], 'NULL');
+  END
+$$;
+
+statement ok
+CREATE FUNCTION g() RETURNS TRIGGER LANGUAGE PLpgSQL AS $$
+  BEGIN
+    RAISE NOTICE 'direct TG_ARGV[0]: %', TG_ARGV[0];
+    RAISE NOTICE 'via helper: %', read_args(TG_ARGV);
+    RETURN NEW;
+  END
+$$;
+
+statement ok
+CREATE TRIGGER foo BEFORE INSERT ON xy FOR EACH ROW EXECUTE FUNCTION g('helper_one', 'helper_two');
 
-# The setting only disallows creating the trigger - not executing it if it was
-# already created.
 query T noticetrace
-INSERT INTO xy VALUES (3, 4);
+INSERT INTO xy VALUES (102, 202);
 ----
-NOTICE: g()
-NOTICE: TG_ARGV[1]: foo
+NOTICE: direct TG_ARGV[0]: helper_one
+NOTICE: via helper: arr[0]=helper_one, arr[1]=helper_two
 
 statement ok
 DROP TRIGGER foo ON xy;
 
 statement ok
 DROP FUNCTION g;
 
+statement ok
+DROP FUNCTION read_args;
+
 subtest end
 
 subtest regression_146889

pkg/sql/logictest/testdata/logic_test/information_schema

@@ -3752,7 +3752,6 @@ WHERE
 ORDER BY variable
 ----
 variable                                                         value
-allow_create_trigger_function_with_argv_references               off
 allow_ordinal_column_references                                  off
 allow_role_memberships_to_change_during_transaction              off
 allow_unsafe_internals                                           off

pkg/sql/logictest/testdata/logic_test/pg_catalog

@@ -3023,7 +3023,6 @@ WHERE
 ORDER BY name
 ----
 name                                                             setting             category  short_desc  extra_desc  vartype
-allow_create_trigger_function_with_argv_references               off                 NULL      NULL        NULL        string
 allow_ordinal_column_references                                  off                 NULL      NULL        NULL        string
 allow_role_memberships_to_change_during_transaction              off                 NULL      NULL        NULL        string
 allow_unsafe_internals                                           off                 NULL      NULL        NULL        string
@@ -3276,7 +3275,6 @@ WHERE
 ORDER BY name
 ----
 name                                                             setting             unit  context  enumvals  boot_val            reset_val
-allow_create_trigger_function_with_argv_references               off                 NULL  user     NULL      off                 off
 allow_ordinal_column_references                                  off                 NULL  user     NULL      off                 off
 allow_role_memberships_to_change_during_transaction              off                 NULL  user     NULL      off                 off
 allow_unsafe_internals                                           off                 NULL  user     NULL      off                 off
@@ -3513,7 +3511,6 @@ query TTTTTT colnames
 SELECT name, source, min_val, max_val, sourcefile, sourceline FROM pg_catalog.pg_settings ORDER BY name
 ----
 name                                                             source  min_val  max_val  sourcefile  sourceline
-allow_create_trigger_function_with_argv_references               NULL    NULL     NULL     NULL        NULL
 allow_ordinal_column_references                                  NULL    NULL     NULL     NULL        NULL
 allow_role_memberships_to_change_during_transaction              NULL    NULL     NULL     NULL        NULL
 allow_unsafe_internals                                           NULL    NULL     NULL     NULL        NULL

pkg/sql/logictest/testdata/logic_test/show_source

@@ -37,7 +37,6 @@ WHERE
 ORDER BY variable
 ----
 variable                                                         value
-allow_create_trigger_function_with_argv_references               off
 allow_ordinal_column_references                                  off
 allow_role_memberships_to_change_during_transaction              off
 allow_unsafe_internals                                           off

pkg/sql/opt/optbuilder/create_trigger.go

@@ -225,12 +225,6 @@ func (b *Builder) buildFunctionForTrigger(
 		paramColName := funcParamColName(param.name, i)
 		col := b.synthesizeColumn(funcScope, paramColName, param.typ, nil /* expr */, nil /* scalar */)
 		col.setParamOrd(i)
-		if i == triggerArgvColIdx {
-			// Due to #135311, we disallow references to the TG_ARGV param for now.
-			if !b.evalCtx.SessionData().AllowCreateTriggerFunctionWithArgvReferences {
-				col.resolveErr = unimplementedArgvErr
-			}
-		}
 	}
 
 	// Now that the transition relations and table type are known, fully build and
@@ -336,10 +330,6 @@ var triggerFuncStaticParams = []routineParam{
 	{name: "tg_argv", typ: types.StringArray, class: tree.RoutineParamIn},
 }
 
-// The trigger function parameters consist of OLD and NEW, followed by the
-// static parameters. The TG_ARGV parameter is last.
-var triggerArgvColIdx = len(triggerFuncStaticParams) + 1
-
 const triggerColNew = "new"
 const triggerColOld = "old"
 
@@ -379,6 +369,4 @@ var (
 		"column lists are not yet supported for triggers")
 	unimplementedViewTriggerErr = unimplemented.NewWithIssue(135658,
 		"triggers on views are not yet supported")
-	unimplementedArgvErr = unimplemented.NewWithIssue(135311,
-		"referencing the TG_ARGV trigger function parameter is not yet supported")
 )

pkg/sql/opt/optbuilder/plpgsql.go

@@ -2222,12 +2222,6 @@ func (b *plpgsqlBuilder) makeContinuation(conName string) continuation {
 		// continuation UDFs.
 		paramOrd := len(params)
 		col.setParamOrd(paramOrd)
-		if b.ob.insideFuncDef && b.options.isTriggerFn && paramOrd == triggerArgvColIdx {
-			// Due to #135311, we disallow references to the TG_ARGV param for now.
-			if !b.ob.evalCtx.SessionData().AllowCreateTriggerFunctionWithArgvReferences {
-				col.resolveErr = unimplementedArgvErr
-			}
-		}
 		params = append(params, col.id)
 	}
 	// Invariant: the variables of a child block always follow those of a parent
@@ -2377,9 +2371,7 @@ func (b *plpgsqlBuilder) makeContinuationArgs(con *continuation, s *scope) memo.
 		block := &b.blocks[i]
 		for _, name := range block.vars {
 			_, source, _, err := s.FindSourceProvidingColumn(b.ob.ctx, name)
-			if err != nil && !errors.Is(err, unimplementedArgvErr) {
-				// Swallow unimplementedArgvErr, since it's ok to reference the TG_ARGV
-				// parameter when calling a continuation.
+			if err != nil {
 				panic(err)
 			}
 			args = append(args, b.ob.factory.ConstructVariable(source.(*scopeColumn).id))

pkg/sql/opt/optbuilder/trigger.go

@@ -234,6 +234,7 @@ func (mb *mutationBuilder) buildTriggerFunctionArgs(
 	tgTableSchema := tree.NewDString(schema.Name().Schema())
 	tgNumArgs := tree.NewDInt(tree.DInt(len(trigger.FuncArgs())))
 	tgArgV := tree.NewDArray(types.String)
+	tgArgV.SetZeroIndexed()
 	for _, arg := range trigger.FuncArgs() {
 		err = tgArgV.Append(arg)
 		if err != nil {
@@ -700,6 +701,7 @@ func (tb *rowLevelAfterTriggerBuilder) Build(
 				tgName := tree.NewDName(string(trigger.Name()))
 				tgNumArgs := tree.NewDInt(tree.DInt(len(trigger.FuncArgs())))
 				tgArgV := tree.NewDArray(types.String)
+				tgArgV.SetZeroIndexed()
 				for _, arg := range trigger.FuncArgs() {
 					err = tgArgV.Append(arg)
 					if err != nil {

pkg/sql/sem/tree/datum.go

@@ -4771,6 +4771,10 @@ type DArray struct {
 
 	// customOid, if non-0, is the oid of this array datum.
 	customOid oid.Oid
+
+	// zeroIndexed, if true, makes FirstIndex() return 0 instead of 1.
+	// This is used for TG_ARGV to match PostgreSQL's 0-indexed behavior.
+	zeroIndexed bool
 }
 
 // NewDArray returns a DArray containing elements of the specified type.
@@ -4883,13 +4887,22 @@ func (d *DArray) IsComposite() bool {
 // which are 1-indexed, and 0 for the special Postgers vector types which are
 // 0-indexed.
 func (d *DArray) FirstIndex() int {
+	if d.zeroIndexed {
+		return 0
+	}
 	switch d.customOid {
 	case oid.T_int2vector, oid.T_oidvector:
 		return 0
 	}
 	return 1
 }
 
+// SetZeroIndexed marks this array as 0-indexed. This is used for TG_ARGV
+// to match PostgreSQL's 0-indexed behavior.
+func (d *DArray) SetZeroIndexed() {
+	d.zeroIndexed = true
+}
+
 // Compare implements the Datum interface.
 func (d *DArray) Compare(ctx context.Context, cmpCtx CompareContext, other Datum) (int, error) {
 	if other == DNull {

pkg/sql/sessiondatapb/local_only_session_data.proto

@@ -644,11 +644,7 @@ message LocalOnlySessionData {
   // OptimizerUseDeleteRangeFastPath, when true, indicates that the optimizer
   // should try to use the delete range fast-path when possible.
   bool optimizer_use_delete_range_fast_path = 164;
-  // AllowCreateTriggerFunctionWithArgvReferences, when true, allows triggers to
-  // be created with trigger functions that use the TG_ARGV parameter even
-  // though it currently doesn't have Postgres-compatible 0-based indexing
-  // behavior.
-  bool allow_create_trigger_function_with_argv_references = 165;
+  reserved 165;
   // CreateTableWithSchemaLocked, when true will create tables as schema_locked
   // by default.
   bool create_table_with_schema_locked = 166;

pkg/sql/sessionmutator/mutator.go

@@ -1031,10 +1031,6 @@ func (m *SessionDataMutator) SetOptimizerUseDeleteRangeFastPath(val bool) {
 	m.Data.OptimizerUseDeleteRangeFastPath = val
 }
 
-func (m *SessionDataMutator) SetAllowCreateTriggerFunctionWithArgvReferences(val bool) {
-	m.Data.AllowCreateTriggerFunctionWithArgvReferences = val
-}
-
 func (m *SessionDataMutator) SetCreateTableWithSchemaLocked(val bool) {
 	m.Data.CreateTableWithSchemaLocked = val
 }