Fix misleading iceberg catalog option error messages

The FDW validator in pg_lake_iceberg used error messages that said
options were "only valid for writable catalog=\"rest\"" when they are
actually valid for read-only external catalog tables (both catalog=rest
and catalog=object_store). Similarly, "required for catalog=\"rest\""
omitted that the same options are required for catalog=object_store
read-only tables.

Changes:
- Reword all "only valid for" / "required for" error messages for
  catalog_name, catalog_namespace, catalog_table_name, and read_only
  to accurately describe which catalog types they apply to.
- Remove a dead "catalog_namespace required" check: it only applied to
  REST read-only (object_store excluded), but the REST ALTER whitelist
  never allows touching catalog_namespace and CREATE auto-fills a
  default, so the path was unreachable.
- Clean up stale copy-pasted comments in ProcessAlterTable that
  referenced "catalog_table_name only" in the object_store branch and
  add a comment explaining the validator interaction.
- Add a comment in option.c noting that the remaining "required"
  checks act as a safety net for ALTER DROP scenarios.

Tests:
- test_iceberg_catalog_option_validation_errors (test_writable_iceberg.py):
  exercises the four "only valid for read-only external catalog tables"
  rejection paths without needing external infrastructure.
- test_object_store_read_only_alter_drop_required_options
  (test_object_store_catalog.py): exercises the two "required" paths
  via ALTER FOREIGN TABLE OPTIONS (DROP ...) on an object_store
  read-only table.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Author: sfc-gh-npuka

pg_lake_table/src/ddl/alter_table.c

@@ -248,20 +248,18 @@ ProcessAlterTable(ProcessUtilityParams * processUtilityParams, void *arg)
 	}
 
 	/*
-	 * For read-only rest catalog iceberg tables, we only allow changing
-	 * catalog_table_name option. All other operations or any other DDLs are
-	 * disallowed.
+	 * For read-only external catalog iceberg tables, we only allow changing
+	 * a subset of catalog options. All other operations are disallowed.
+	 * After the whitelist check passes, we return false so PostgreSQL
+	 * processes the ALTER normally (which invokes the FDW validator on
+	 * the merged option set).
 	 */
 	if (HasOnlyCatalogAlterTableOptions(alterStmt))
 	{
 		IcebergCatalogType icebergCatalogType = GetIcebergCatalogType(relationId);
 
 		if (icebergCatalogType == REST_CATALOG_READ_ONLY)
 		{
-			/*
-			 * We currently only allow changing catalog_table_name option for
-			 * read-only rest catalog iceberg tables.
-			 */
 			List	   *allowedOptions = list_make1("catalog_table_name");
 
 			ErrorIfUnsupportedTableOptionChange(alterStmt, allowedOptions);
@@ -270,10 +268,6 @@ ProcessAlterTable(ProcessUtilityParams * processUtilityParams, void *arg)
 		}
 		else if (icebergCatalogType == OBJECT_STORE_READ_ONLY)
 		{
-			/*
-			 * We currently only allow changing catalog_table_name option for
-			 * read-only rest catalog iceberg tables.
-			 */
 			List	   *allowedOptions = list_make3("catalog_table_name", "catalog_namespace", "catalog_name");
 
 			ErrorIfUnsupportedTableOptionChange(alterStmt, allowedOptions);

pg_lake_table/src/fdw/option.c

@@ -908,30 +908,29 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
 		!(icebergCatalogType == REST_CATALOG_READ_ONLY || icebergCatalogType == OBJECT_STORE_READ_ONLY))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("\"read_only\" option is only valid for catalog=\"rest\"")));
+				 errmsg("\"read_only\" option is only valid for catalog=\"rest\" or catalog=\"object_store\"")));
 
 	if (catalog == ForeignTableRelationId)
 	{
 		if (icebergCatalogType == REST_CATALOG_READ_ONLY || icebergCatalogType == OBJECT_STORE_READ_ONLY)
 		{
 			/*
-			 * catalog_namespace, catalog_table_name and catalog_name is
-			 * required for catalog=rest
+			 * catalog_name and catalog_table_name are required for
+			 * read-only external catalog tables, i.e. catalog=rest and catalog=object_store
+			 *
+			 * On CREATE, ProcessCreateIcebergTableFromForeignTableStmt
+			 * auto-fills defaults for these options, so these checks
+			 * act as a safety net for ALTER DROP scenarios.
 			 */
 			if (!catalogName)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("\"catalog_name\" option is required for catalog=\"rest\"")));
-
-			if (!catalogNamespace && icebergCatalogType != OBJECT_STORE_READ_ONLY)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("\"catalog_namespace\" option is required for catalog=\"rest\"")));
+						 errmsg("\"catalog_name\" option is required for read-only external catalog tables")));
 
 			if (!catalogTableName)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("\"catalog_table_name\" option is required for catalog=\"rest\"")));
+						 errmsg("\"catalog_table_name\" option is required for read-only external catalog tables")));
 		}
 		else
 		{
@@ -941,17 +940,17 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
 			if (catalogNamespace)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("\"catalog_namespace\" option is only valid for writable catalog=\"rest\"")));
+						 errmsg("\"catalog_namespace\" option is only valid for read-only external catalog tables")));
 
 			if (catalogTableName)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("\"catalog_table_name\" option is only valid for writable catalog=\"rest\"")));
+						 errmsg("\"catalog_table_name\" option is only valid for read-only external catalog tables")));
 
 			if (catalogName)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						 errmsg("\"catalog_name\" option is only valid for writable catalog=\"rest\"")));
+						 errmsg("\"catalog_name\" option is only valid for read-only external catalog tables")));
 
 		}
 	}

pg_lake_table/tests/pytests/test_object_store_catalog.py

@@ -127,6 +127,47 @@ def test_read_only_object_store_with_non_existing_options(
     pg_conn.commit()
 
 
+def test_object_store_read_only_alter_drop_required_options(
+    pg_conn, s3, extension, with_default_location, adjust_object_store_settings
+):
+    schema = "test_alter_drop_required"
+    run_command(f"CREATE SCHEMA {schema}", pg_conn)
+
+    run_command(
+        f"CREATE TABLE {schema}.wrt_tbl(a INT) USING iceberg WITH (catalog='object_store')",
+        pg_conn,
+    )
+    pg_conn.commit()
+    wait_until_object_store_writable_table_pushed(pg_conn, schema, "wrt_tbl")
+
+    run_command(
+        f"CREATE TABLE {schema}.ro_tbl() USING iceberg WITH (catalog='object_store', read_only=True, catalog_table_name='wrt_tbl')",
+        pg_conn,
+    )
+    pg_conn.commit()
+
+    # dropping catalog_name should fail — it is required
+    error = run_command(
+        f"ALTER FOREIGN TABLE {schema}.ro_tbl OPTIONS (DROP catalog_name)",
+        pg_conn,
+        raise_error=False,
+    )
+    assert '"catalog_name" option is required for read-only external catalog tables' in str(error)
+    pg_conn.rollback()
+
+    # dropping catalog_table_name should fail — it is required
+    error = run_command(
+        f"ALTER FOREIGN TABLE {schema}.ro_tbl OPTIONS (DROP catalog_table_name)",
+        pg_conn,
+        raise_error=False,
+    )
+    assert '"catalog_table_name" option is required for read-only external catalog tables' in str(error)
+    pg_conn.rollback()
+
+    run_command(f"DROP SCHEMA {schema} CASCADE", pg_conn)
+    pg_conn.commit()
+
+
 # basic flow for object store catalog tables
 # changes on the source is reflected on the target
 def test_read_only_object_store_read_write(

pg_lake_table/tests/pytests/test_writable_iceberg.py

@@ -120,6 +120,55 @@ def test_writable_iceberg_create_wrong_option(pg_conn, extension):
     pg_conn.commit()
 
 
+def test_iceberg_catalog_option_validation_errors(pg_conn, extension):
+    schema = "test_catalog_option_validation"
+    run_command(f"CREATE SCHEMA {schema}", pg_conn)
+    pg_conn.commit()
+
+    # read_only is only valid for catalog="rest" or catalog="object_store"
+    error = run_command(
+        f"""CREATE FOREIGN TABLE {schema}.ft (id int) SERVER pg_lake_iceberg
+            OPTIONS (location 's3://bucket/path', read_only 'true')""",
+        pg_conn,
+        raise_error=False,
+    )
+    assert '"read_only" option is only valid for catalog="rest" or catalog="object_store"' in str(error)
+    pg_conn.rollback()
+
+    # catalog_name is only valid for read-only external catalog tables
+    error = run_command(
+        f"""CREATE FOREIGN TABLE {schema}.ft (id int) SERVER pg_lake_iceberg
+            OPTIONS (location 's3://bucket/path', catalog_name 'cat')""",
+        pg_conn,
+        raise_error=False,
+    )
+    assert '"catalog_name" option is only valid for read-only external catalog tables' in str(error)
+    pg_conn.rollback()
+
+    # catalog_namespace is only valid for read-only external catalog tables
+    error = run_command(
+        f"""CREATE FOREIGN TABLE {schema}.ft (id int) SERVER pg_lake_iceberg
+            OPTIONS (location 's3://bucket/path', catalog_namespace 'ns')""",
+        pg_conn,
+        raise_error=False,
+    )
+    assert '"catalog_namespace" option is only valid for read-only external catalog tables' in str(error)
+    pg_conn.rollback()
+
+    # catalog_table_name is only valid for read-only external catalog tables
+    error = run_command(
+        f"""CREATE FOREIGN TABLE {schema}.ft (id int) SERVER pg_lake_iceberg
+            OPTIONS (location 's3://bucket/path', catalog_table_name 'tbl')""",
+        pg_conn,
+        raise_error=False,
+    )
+    assert '"catalog_table_name" option is only valid for read-only external catalog tables' in str(error)
+    pg_conn.rollback()
+
+    run_command(f"DROP SCHEMA {schema} CASCADE", pg_conn)
+    pg_conn.commit()
+
+
 def test_writable_iceberg_table_failure(
     installcheck,
     install_iceberg_to_duckdb,