spock.sub_alter_options: validate input, skip no-op restarts, add regression tests

Harden the sub_alter_options() implementation in three ways:

* No-op optimisation: track a `changed` flag through the JSONB parsing
  loop.  alter_subscription() — which sends SIGTERM to the apply worker
  at commit — is only called when at least one option was actually
  modified.  An empty object '{}' now returns false without disturbing
  the apply worker.

* Return value: the function now returns false when nothing changed and
  true when at least one subscription field was updated, giving callers
  a reliable signal about whether a worker restart is coming.

* Input validation: forward_origins elements are checked against the
  only supported sentinel value "all".  Any other string raises
  INVALID_PARAMETER_VALUE with a hint, replacing the previous TODO
  comment.

Add tests/regress/sql/alter_options.sql covering: baseline state
snapshot, no-op '{}', each option individually (forward_origins,
apply_delay, skip_schema), a multi-option call, baseline restore, and
all expected error paths (non-object JSON, unknown key, wrong value
types, invalid interval, non-existent subscription, invalid
forward_origins value).

Author: danolivo

Makefile

@@ -54,7 +54,7 @@ REGRESS = preseed infofuncs init_fail init preseed_check basic conflict_secondar
 		  excluded_schema conflict_stat \
 		  toasted replication_set exception_row_capture matview bidirectional primary_key \
 		  interfaces foreign_key copy sequence triggers parallel functions row_filter \
-		  row_filter_sampling att_list column_filter apply_delay \
+		  row_filter_sampling att_list column_filter apply_delay alter_options \
 		  extended node_origin_cascade multiple_upstreams tuple_origin autoddl \
 		  sync_event sync_table generated_columns spill_transaction read_only drop
 

src/spock_functions.c

@@ -947,6 +947,7 @@ spock_alter_subscription_options(PG_FUNCTION_ARGS)
 	JsonbIterator *it;
 	JsonbIteratorToken r;
 	JsonbValue	v;
+	bool		changed = false;
 
 	/* XXX: Only used for locking purposes. */
 	(void) get_local_node(true, false);
@@ -984,20 +985,33 @@ spock_alter_subscription_options(PG_FUNCTION_ARGS)
 
 			while ((r = JsonbIteratorNext(&it, &v, false)) != WJB_END_ARRAY)
 			{
+				char   *elem;
+
 				if (r != WJB_ELEM || v.type != jbvString)
 					ereport(ERROR,
 							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 							 errmsg("option \"%s\" must contain only strings",
 									key)));
-				/* TODO: further commits should perform a precheck of the input */
-				result = lappend(result,
-								 pnstrdup(v.val.string.val, v.val.string.len));
+
+				elem = pnstrdup(v.val.string.val, v.val.string.len);
+
+				/* forward_origins only accepts the sentinel value "all" */
+				if (strcmp(key, "forward_origins") == 0 &&
+					strcmp(elem, "all") != 0)
+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value \"%s\" for option \"forward_origins\"",
+									elem),
+							 errhint("The only supported value is \"all\".")));
+
+				result = lappend(result, elem);
 			}
 
 			if (strcmp(key, "forward_origins") == 0)
 				sub->forward_origins = result;
 			else
 				sub->skip_schema = result;
+			changed = true;
 		}
 		else if (strcmp(key, "apply_delay") == 0)
 		{
@@ -1015,6 +1029,7 @@ spock_alter_subscription_options(PG_FUNCTION_ARGS)
 									CStringGetDatum(delay_str),
 									ObjectIdGetDatum(InvalidOid),
 									Int32GetDatum(-1)));
+			changed = true;
 		}
 		else
 		{
@@ -1026,9 +1041,10 @@ spock_alter_subscription_options(PG_FUNCTION_ARGS)
 		}
 	}
 
-	alter_subscription(sub);
+	if (changed)
+		alter_subscription(sub);
 
-	PG_RETURN_BOOL(true);
+	PG_RETURN_BOOL(changed);
 }
 
 /*