Use iceberg_catalog server OID as canonical REST catalog identity

Now that every REST catalog -- built-in and user-created -- is backed by
a real iceberg_catalog foreign server, the server's OID is a stable,
unambiguous identifier.  This commit pivots the in-memory machinery off
case-insensitive name comparison and onto OID equality.

* RestCatalogOptions gains a leading `Oid serverOid` field, populated in
  BuildRestCatalogOptionsFromServer from server->serverid and propagated
  by CopyRestCatalogOptions.  The struct comment is updated to call out
  that `catalog` is now purely a user-facing label kept around for error
  messages, and that `serverOid` is the canonical identity.

* The per-catalog OAuth token cache is rekeyed from a NAMEDATALEN char
  buffer to sizeof(Oid).  BuildTokenCacheKey is removed; the lookup site
  now passes &opts->serverOid directly, with a defensive
  Assert(OidIsValid(opts->serverOid)) so a future caller that forgets to
  resolve options cannot silently funnel every catalog into the same
  cache slot.  InvalidateRestTokenCache keeps its existing full-flush
  behavior on any pg_foreign_server change; targeted invalidation is
  not worth the per-entry bookkeeping at the rate ALTER SERVER actually
  happens.

* The cross-transaction "same REST catalog throughout" guard in
  BindRelationToXactRestCatalog and the belt-and-suspenders branch of
  RecordRestCatalogRequestInTx now compare serverOid directly instead
  of pg_strcasecmp'ing catalog names.  The two user-facing names are
  still surfaced in errdetail so the message remains in the terms the
  user typed.  This also closes the corner case where the same physical
  server is referenced via different casings in successive statements:
  they now collapse to the same OID and are treated as one catalog.

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

Author: sfc-gh-npuka

pg_lake_iceberg/include/pg_lake/rest_catalog/rest_catalog.h

@@ -36,16 +36,25 @@ extern int	RestCatalogAuthType;
 extern bool RestCatalogEnableVendedCredentials;
 
 /*
- * Resolved REST catalog connection options.  For the built-in 'rest'
- * catalog the fields come entirely from GUC settings.  For user-created
- * catalogs (CREATE SERVER ... FOREIGN DATA WRAPPER iceberg_catalog) the
- * server options override the GUC defaults.
+ * Resolved REST catalog connection options.  All REST catalogs --
+ * built-in ('rest') and user-created (CREATE SERVER ... FOREIGN DATA
+ * WRAPPER iceberg_catalog) -- are backed by a real pg_foreign_server
+ * row; ApplyGUCDefaults populates the defaults, ApplyServerOptionOverrides
+ * layers on any per-server options.
+ *
+ * The canonical identity of a catalog is `serverOid` (the OID of the
+ * iceberg_catalog server row).  Use it for in-memory equality, token
+ * cache keys, and syscache-driven invalidation.  `catalog` stores the
+ * user-visible short name (e.g. 'rest', 'my_polaris') purely for error
+ * messages.
  */
 typedef struct RestCatalogOptions
 {
-	char	   *catalog;		/* catalog name, used for token cache keying;
-								 * can be 'rest' or a user-created server name
-								 * of TYPE 'rest' */
+	Oid			serverOid;		/* iceberg_catalog server OID; canonical
+								 * identity, never InvalidOid for resolved
+								 * opts */
+	char	   *catalog;		/* short user-facing name; used in error
+								 * messages, never for equality */
 	char	   *host;
 	char	   *oauthHostPath;
 	char	   *clientId;

pg_lake_iceberg/src/rest_catalog/rest_catalog.c

@@ -68,14 +68,12 @@ int			RestCatalogAuthType = REST_CATALOG_AUTH_TYPE_OAUTH2;
 bool		RestCatalogEnableVendedCredentials = true;
 
 /*
- * Per-rest-catalog token cache. Keyed by catalog.
- * Should always be accessed via GetRestCatalogAccessToken()
+ * Per-rest-catalog token cache, keyed by the iceberg_catalog server OID.
+ * Should always be accessed via GetRestCatalogAccessToken().
  */
-#define TOKEN_CACHE_KEY_LEN NAMEDATALEN
-
 typedef struct RestCatalogTokenCacheEntry
 {
-	char		key[TOKEN_CACHE_KEY_LEN];
+	Oid			serverOid;		/* hash key */
 	char	   *accessToken;
 	TimestampTz accessTokenExpiry;
 }			RestCatalogTokenCacheEntry;
@@ -527,8 +525,7 @@ ValidateIcebergCatalogServerDDL(ProcessUtilityParams * processUtilityParams,
 		 * Renaming an iceberg_catalog server is blocked because dependent
 		 * iceberg tables store the server name as a string option
 		 * (catalog='<name>') in pg_foreign_table.ftoptions.  A rename would
-		 * silently break those references.  This covers both user-created
-		 * servers and the three built-in ones.
+		 * silently break those references.
 		 */
 		ForeignServer *server = GetForeignServerByName(strVal(stmt->object),
 													   true);
@@ -694,6 +691,7 @@ BuildRestCatalogOptionsFromServer(const char *serverName,
 
 	RestCatalogOptions *opts = palloc0(sizeof(RestCatalogOptions));
 
+	opts->serverOid = server->serverid;
 	opts->catalog = pstrdup(userVisibleCatalog);
 	ApplyGUCDefaults(opts);
 	ApplyServerOptionOverrides(opts, server);
@@ -748,6 +746,7 @@ CopyRestCatalogOptions(MemoryContext dst, const RestCatalogOptions * src)
 	MemoryContext oldctx = MemoryContextSwitchTo(dst);
 	RestCatalogOptions *copy = palloc0(sizeof(RestCatalogOptions));
 
+	copy->serverOid = src->serverOid;
 	copy->catalog = pstrdup(src->catalog);
 	copy->host = pstrdup(src->host);
 	copy->oauthHostPath = src->oauthHostPath ? pstrdup(src->oauthHostPath) : NULL;
@@ -1230,29 +1229,17 @@ ReportHTTPError(HttpResult httpResult, int level)
 }
 
 
-/*
- * Build a cache key for the per-catalog token cache.
- */
-static void
-BuildTokenCacheKey(char *key, const RestCatalogOptions * opts)
-{
-	Assert(opts->catalog != NULL);
-	MemSet(key, 0, TOKEN_CACHE_KEY_LEN);
-	strlcpy(key, opts->catalog, TOKEN_CACHE_KEY_LEN);
-}
-
-
 /*
  * Syscache invalidation callback for pg_foreign_server changes.
  * Any ALTER/DROP SERVER blows away the entire token cache so stale
  * credentials are never reused.  The cache is rebuilt lazily on the
  * next token lookup.
  *
  * We ignore hashvalue and reset the whole cache rather than selectively
- * invalidating a single server's entry (as postgres_fdw does).  With a
- * handful of servers and infrequent ALTER SERVER, the cost of a few
- * extra OAuth round-trips is negligible compared to the complexity of
- * keying the cache by OID for selective invalidation.
+ * invalidating a single server's entry.  With a handful of servers and
+ * infrequent ALTER SERVER, the cost of a few extra OAuth round-trips is
+ * negligible compared to the complexity of tracking per-entry hash
+ * values for targeted invalidation.
  */
 static void
 InvalidateRestTokenCache(Datum arg, int cacheid, uint32 hashvalue)
@@ -1297,7 +1284,7 @@ InitTokenCacheIfNeeded(void)
 	HASHCTL		ctl;
 
 	memset(&ctl, 0, sizeof(ctl));
-	ctl.keysize = TOKEN_CACHE_KEY_LEN;
+	ctl.keysize = sizeof(Oid);
 	ctl.entrysize = sizeof(RestCatalogTokenCacheEntry);
 	ctl.hcxt = RestTokenCacheCtx;
 
@@ -1309,7 +1296,7 @@ InitTokenCacheIfNeeded(void)
 
 /*
  * Gets an access token from rest catalog. Caches the token per catalog
- * (keyed by catalog) until it is about to expire.
+ * (keyed by iceberg_catalog server OID) until it is about to expire.
  */
 static char *
 GetRestCatalogAccessToken(RestCatalogOptions * opts, bool forceRefreshToken)
@@ -1319,15 +1306,19 @@ GetRestCatalogAccessToken(RestCatalogOptions * opts, bool forceRefreshToken)
 				(errcode(ERRCODE_INTERNAL_ERROR),
 				 errmsg("REST catalog options must not be NULL when fetching access token")));
 
-	InitTokenCacheIfNeeded();
-
-	char		cacheKey[TOKEN_CACHE_KEY_LEN];
+	/*
+	 * Every resolved RestCatalogOptions originates from
+	 * BuildRestCatalogOptionsFromServer, which always sets serverOid. A
+	 * missing OID would silently funnel every catalog into the same cache
+	 * slot, so trap it loudly here.
+	 */
+	Assert(OidIsValid(opts->serverOid));
 
-	BuildTokenCacheKey(cacheKey, opts);
+	InitTokenCacheIfNeeded();
 
 	bool		found = false;
 	RestCatalogTokenCacheEntry *entry =
-		hash_search(RestCatalogTokenCache, cacheKey, HASH_ENTER, &found);
+		hash_search(RestCatalogTokenCache, &opts->serverOid, HASH_ENTER, &found);
 
 	if (!found)
 	{

pg_lake_table/src/transaction/track_iceberg_metadata_changes.c

@@ -656,13 +656,14 @@ BindRelationToXactRestCatalog(Oid relationId)
 	}
 
 	/*
-	 * Both sides of the comparison are the canonical catalog name (lowercase
-	 * "rest" for the built-in catalog, server name as stored in
-	 * pg_foreign_server for user-defined ones).  pg_strcasecmp matches the
-	 * casing rules PostgreSQL applies to identifier resolution.
+	 * Identity is the iceberg_catalog server OID, not the user-typed name.
+	 * Two requests for the same physical server -- whether the user spelled
+	 * the catalog as 'rest', 'REST', or as the underlying built-in server
+	 * name on different statements -- collapse to the same OID and are
+	 * treated as one catalog.  The user-facing names are still reported in
+	 * the error message.
 	 */
-	if (pg_strcasecmp(PgLakeXactRestCatalog->catalogOpts->catalog,
-					  resolvedOpts->catalog) != 0)
+	if (PgLakeXactRestCatalog->catalogOpts->serverOid != resolvedOpts->serverOid)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot modify tables from different REST catalogs "
@@ -715,9 +716,11 @@ RecordRestCatalogRequestInTx(Oid relationId, RestCatalogOperationType operationT
 		 * bind through BindRelationToXactRestCatalog() at statement time or
 		 * populate catalogOpts via the branch above, so in practice we never
 		 * reach here with a mismatched catalog.  Kept as a last line of
-		 * defense for any future code path that forgets to do so.
+		 * defense for any future code path that forgets to do so.  See the
+		 * companion comment in BindRelationToXactRestCatalog for why identity
+		 * is the server OID, not the user-typed name.
 		 */
-		else if (pg_strcasecmp(PgLakeXactRestCatalog->catalogOpts->catalog, resolvedOpts->catalog) != 0)
+		else if (PgLakeXactRestCatalog->catalogOpts->serverOid != resolvedOpts->serverOid)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("cannot modify tables from different REST catalogs "