Allow extension owned rest catalog to alter server options

This means any type 'rest' server (extension or user-owned)
will use GUC options as default, and change any option to the ones
set on the server.

This eliminates the need of the function IsRestCatalogOwnedByUsers
Instead we just have IsRestCatalog function.

This also eliminates the need of GetRestCatalogConnectionFromGUCs
Instead we just have GetRestCatalogConnectionFromServer - this
function uses GUC values as defaults for all type 'rest' servers.

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Author: sfc-gh-npuka

pg_lake_engine/include/pg_lake/util/catalog_type.h

@@ -62,5 +62,4 @@ extern PGDLLEXPORT bool HasRestCatalogTableOption(List *options);
 extern PGDLLEXPORT bool HasObjectStoreCatalogTableOption(List *options);
 extern PGDLLEXPORT bool HasReadOnlyOption(List *options);
 extern PGDLLEXPORT bool IsCatalogOwnedByExtension(const char *catalog);
-extern PGDLLEXPORT bool IsRestCatalogOwnedByExtension(const char *catalog);
-extern PGDLLEXPORT bool IsRestCatalogOwnedByUsers(List *options);
+extern PGDLLEXPORT bool IsRestCatalog(const char *catalog);

pg_lake_engine/src/utils/catalog_type.c

@@ -77,13 +77,7 @@ HasRestCatalogTableOption(List *options)
 {
 	char	   *catalog = GetStringOption(options, "catalog", false);
 
-	if (catalog == NULL)
-		return false;
-
-	if (IsRestCatalogOwnedByExtension(catalog))
-		return true;
-
-	return IsRestCatalogOwnedByUsers(options);
+	return IsRestCatalog(catalog);
 }
 
 
@@ -113,46 +107,32 @@ HasReadOnlyOption(List *options)
 }
 
 
-/*
- * IsRestCatalogOwnedByExtension returns true if the catalog name matches
- * the extension-owned 'rest' catalog literal.
- */
-bool
-IsRestCatalogOwnedByExtension(const char *catalog)
-{
-	return pg_strncasecmp(catalog, REST_CATALOG_NAME, strlen(REST_CATALOG_NAME)) == 0;
-}
-
-
 /*
  * IsCatalogOwnedByExtension returns true if the catalog name is one of the
  * extension-owned literals: 'rest', 'object_store', or 'postgres'.
  */
 bool
 IsCatalogOwnedByExtension(const char *catalog)
 {
-	return IsRestCatalogOwnedByExtension(catalog) ||
+	return pg_strncasecmp(catalog, REST_CATALOG_NAME, strlen(REST_CATALOG_NAME)) == 0 ||
 		pg_strncasecmp(catalog, OBJECT_STORE_CATALOG_NAME, strlen(OBJECT_STORE_CATALOG_NAME)) == 0 ||
 		pg_strncasecmp(catalog, POSTGRES_CATALOG_NAME, strlen(POSTGRES_CATALOG_NAME)) == 0;
 }
 
 
 /*
- * IsRestCatalogOwnedByUsers returns true if the catalog option refers to a
- * ForeignServer created by the user with the iceberg_catalog FDW whose TYPE is 'rest'.
- * Returns false if the catalog is owned by the extension ('rest',
- * 'object_store', 'postgres') or if no matching server is found.
+ * IsRestCatalog returns true if the catalog name identifies a REST catalog.
+ * This includes the extension-owned 'rest' literal and any user-created
+ * iceberg_catalog server whose TYPE is 'rest' (or omitted, defaulting to 'rest').
  */
 bool
-IsRestCatalogOwnedByUsers(List *options)
+IsRestCatalog(const char *catalog)
 {
-	char	   *catalog = GetStringOption(options, "catalog", false);
-
 	if (catalog == NULL)
 		return false;
 
-	if (IsCatalogOwnedByExtension(catalog))
-		return false;
+	if (pg_strncasecmp(catalog, REST_CATALOG_NAME, strlen(REST_CATALOG_NAME)) == 0)
+		return true;
 
 	/* Try to look up a server with this name */
 	bool		missingOK = true;

pg_lake_iceberg/include/pg_lake/rest_catalog/rest_catalog.h

@@ -36,14 +36,13 @@ extern int	RestCatalogAuthType;
 extern bool RestCatalogEnableVendedCredentials;
 
 /*
- * Holds per-server REST catalog connection settings. Can be populated from
- * GUCs (for backward-compatible catalog='rest') or from a ForeignServer
- * created via CREATE SERVER ... FOREIGN DATA WRAPPER iceberg_catalog.
+ * Holds per-server REST catalog connection settings. Populated from the
+ * server options of an iceberg_catalog ForeignServer, with GUC fallback
+ * for any option not explicitly set on the server.
  */
 typedef struct RestCatalogConnectionInfo
 {
-	char	   *serverName;		/* server name for cache keying, NULL for
-								 * GUC-based */
+	char	   *serverName;		/* server name, used for token cache keying */
 	char	   *host;
 	char	   *oauthHostPath;
 	char	   *clientId;
@@ -97,7 +96,6 @@ typedef struct RestCatalogRequest
 #define GET_REST_CATALOG_METADATA_LOCATION "%s/api/catalog/v1/%s/namespaces/%s/tables/%s"
 
 /* Connection info resolution */
-extern PGDLLEXPORT RestCatalogConnectionInfo * GetRestCatalogConnectionFromGUCs(void);
 extern PGDLLEXPORT RestCatalogConnectionInfo * GetRestCatalogConnectionFromServer(const char *serverName);
 extern PGDLLEXPORT RestCatalogConnectionInfo * GetRestCatalogConnectionForRelation(Oid relationId);
 

pg_lake_iceberg/src/rest_catalog/rest_catalog.c

@@ -291,37 +291,20 @@ ProtectExtensionCatalogServersHandler(ProcessUtilityParams *processUtilityParams
 }
 
 
-/*
- * GetRestCatalogConnectionFromGUCs returns a RestCatalogConnectionInfo
- * populated from the current GUC variables. Used for backward-compatible
- * catalog='rest' tables.
- */
-RestCatalogConnectionInfo *
-GetRestCatalogConnectionFromGUCs(void)
-{
-	RestCatalogConnectionInfo *conn = palloc0(sizeof(RestCatalogConnectionInfo));
-
-	conn->serverName = REST_CATALOG_NAME;
-	conn->host = RestCatalogHost;
-	conn->oauthHostPath = RestCatalogOauthHostPath;
-	conn->clientId = RestCatalogClientId;
-	conn->clientSecret = RestCatalogClientSecret;
-	conn->scope = RestCatalogScope;
-	conn->authType = RestCatalogAuthType;
-	conn->enableVendedCredentials = RestCatalogEnableVendedCredentials;
-
-	return conn;
-}
-
-
 /*
  * GetRestCatalogConnectionFromServer returns a RestCatalogConnectionInfo
- * populated from the options of a ForeignServer (non-secret config) and
- * its USER MAPPING (credentials) for the current user.
+ * populated from the options of the named ForeignServer. GUC values are
+ * used as defaults; any option explicitly set on the server overrides the
+ * corresponding GUC.  This applies to both the extension-owned 'rest'
+ * server and user-created iceberg_catalog servers.
  */
 RestCatalogConnectionInfo *
 GetRestCatalogConnectionFromServer(const char *serverName)
 {
+	/* Normalize case-insensitive match to the canonical pre-created name */
+	if (pg_strcasecmp(serverName, REST_CATALOG_NAME) == 0)
+		serverName = REST_CATALOG_NAME;
+
 	ForeignServer *server = GetForeignServerByName(serverName, false);
 	ForeignDataWrapper *fdw = GetForeignDataWrapper(server->fdwid);
 
@@ -335,14 +318,14 @@ GetRestCatalogConnectionFromServer(const char *serverName)
 
 	conn->serverName = pstrdup(serverName);
 
-	/* Set defaults matching the GUC defaults */
-	conn->host = NULL;
-	conn->oauthHostPath = "";
-	conn->clientId = NULL;
-	conn->clientSecret = NULL;
-	conn->scope = "PRINCIPAL_ROLE:ALL";
-	conn->authType = REST_CATALOG_AUTH_TYPE_DEFAULT;
-	conn->enableVendedCredentials = true;
+	/* GUC values serve as defaults; server options override below */
+	conn->host = RestCatalogHost;
+	conn->oauthHostPath = RestCatalogOauthHostPath;
+	conn->clientId = RestCatalogClientId;
+	conn->clientSecret = RestCatalogClientSecret;
+	conn->scope = RestCatalogScope;
+	conn->authType = RestCatalogAuthType;
+	conn->enableVendedCredentials = RestCatalogEnableVendedCredentials;
 
 	ListCell   *lc;
 
@@ -372,7 +355,7 @@ GetRestCatalogConnectionFromServer(const char *serverName)
 			conn->enableVendedCredentials = defGetBoolean(def);
 	}
 
-	if (conn->host == NULL)
+	if (conn->host == NULL || conn->host[0] == '\0')
 		ereport(ERROR,
 				(errcode(ERRCODE_FDW_OPTION_NAME_NOT_FOUND),
 				 errmsg("\"rest_endpoint\" option is required for iceberg_catalog server \"%s\"",
@@ -384,9 +367,9 @@ GetRestCatalogConnectionFromServer(const char *serverName)
 
 /*
  * GetRestCatalogConnectionForRelation returns the REST catalog connection
- * info for the given relation. If the table uses catalog='rest', the
- * connection is built from GUCs. Otherwise, the catalog option is treated
- * as a server name and the connection is built from its options.
+ * info for the given relation. The catalog option value is used as the
+ * server name.  For the extension-owned 'rest' server and user-created
+ * servers alike, server options are read first with GUC fallback.
  */
 RestCatalogConnectionInfo *
 GetRestCatalogConnectionForRelation(Oid relationId)
@@ -399,9 +382,6 @@ GetRestCatalogConnectionForRelation(Oid relationId)
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("catalog option is not set for relation %u", relationId)));
 
-	if (IsRestCatalogOwnedByExtension(catalog))
-		return GetRestCatalogConnectionFromGUCs();
-
 	return GetRestCatalogConnectionFromServer(catalog);
 }
 

pg_lake_iceberg/src/test/rest_catalog.c

@@ -37,7 +37,7 @@ register_namespace_to_rest_catalog(PG_FUNCTION_ARGS)
 	char	   *catalogName = text_to_cstring(PG_GETARG_TEXT_P(0));
 	char	   *namespaceName = text_to_cstring(PG_GETARG_TEXT_P(1));
 
-	RestCatalogConnectionInfo *conn = GetRestCatalogConnectionFromGUCs();
+	RestCatalogConnectionInfo *conn = GetRestCatalogConnectionFromServer(REST_CATALOG_NAME);
 
 	RegisterNamespaceToRestCatalog(conn, catalogName, namespaceName);
 	PG_RETURN_VOID();

pg_lake_table/src/ddl/create_table.c

@@ -720,12 +720,8 @@ ProcessCreateIcebergTableFromForeignTableStmt(ProcessUtilityParams * params)
 		if (hasRestCatalogOption && hasExternalCatalogReadOnlyOption)
 		{
 			char	   *catalogOptionValue = GetStringOption(createStmt->options, "catalog", false);
-			RestCatalogConnectionInfo *conn;
-
-			if (IsRestCatalogOwnedByExtension(catalogOptionValue))
-				conn = GetRestCatalogConnectionFromGUCs();
-			else
-				conn = GetRestCatalogConnectionFromServer(catalogOptionValue);
+			RestCatalogConnectionInfo *conn =
+				GetRestCatalogConnectionFromServer(catalogOptionValue);
 
 			ErrorIfRestNamespaceDoesNotExist(conn, catalogName, catalogNamespace);
 
@@ -936,12 +932,8 @@ ProcessCreateIcebergTableFromForeignTableStmt(ProcessUtilityParams * params)
 		 * etc., but here we need to do it early before the table is created.
 		 */
 		char	   *catalogOptionValue = GetStringOption(createStmt->options, "catalog", false);
-		RestCatalogConnectionInfo *conn;
-
-		if (IsRestCatalogOwnedByExtension(catalogOptionValue))
-			conn = GetRestCatalogConnectionFromGUCs();
-		else
-			conn = GetRestCatalogConnectionFromServer(catalogOptionValue);
+		RestCatalogConnectionInfo *conn =
+			GetRestCatalogConnectionFromServer(catalogOptionValue);
 
 		RegisterNamespaceToRestCatalog(conn, get_database_name(MyDatabaseId),
 									   get_namespace_name(namespaceId));

pg_lake_table/src/fdw/option.c

@@ -742,11 +742,7 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
 		{
 			char	   *icebergCatalogName = defGetString(def);
 
-			/*
-			 * We only accept "rest" and "postgres" for now. If not provided,
-			 * assume "postgres" by default. Don't allow anything.
-			 */
-			if (pg_strncasecmp(icebergCatalogName, REST_CATALOG_NAME, strlen(icebergCatalogName)) == 0)
+			if (IsRestCatalog(icebergCatalogName))
 			{
 				/*
 				 * at this point, we cannot tell whether it's read only or
@@ -757,7 +753,6 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
 			}
 			else if (pg_strncasecmp(icebergCatalogName, OBJECT_STORE_CATALOG_NAME, strlen(icebergCatalogName)) == 0)
 			{
-
 				/*
 				 * at this point, we cannot tell whether it's read only or
 				 * read write. We'll determine that later based on the
@@ -768,20 +763,11 @@ pg_lake_iceberg_validator(PG_FUNCTION_ARGS)
 			else if (pg_strncasecmp(icebergCatalogName, POSTGRES_CATALOG_NAME, strlen(icebergCatalogName)) == 0)
 				icebergCatalogType = POSTGRES_CATALOG;
 			else
-			{
-				/*
-				 * Check if the catalog value refers to an iceberg_catalog
-				 * server. If so, treat it as a REST catalog.
-				 */
-				if (IsRestCatalogOwnedByUsers(options_list))
-					icebergCatalogType = REST_CATALOG_READ_ONLY;
-				else
-					ereport(ERROR,
-							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-							 errmsg("invalid catalog option: %s", icebergCatalogName),
-							 errdetail("Use \"rest\", \"object_store\", \"postgres\", "
-									   "or the name of an iceberg_catalog server.")));
-			}
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("invalid catalog option: %s", icebergCatalogName),
+						 errdetail("Use \"rest\", \"object_store\", \"postgres\", "
+								   "or the name of an iceberg_catalog server.")));
 		}
 		else if (catalog == ForeignTableRelationId && strcmp(def->defname, "read_only") == 0)
 		{