New Catalog Configuration via CREATE SERVER by sfc-gh-npuka · Pull Request #242 · Snowflake-Labs/pg_lake

sfc-gh-npuka · 2026-03-02T13:05:13Z

Note to reviewer: Let's assume we can save client secrets as server options for now. Easier to review commit by commit as each commit has its own description. The credentials are part of USER MAPPING in this PR #255

How users can create REST catalogs

CREATE SERVER my_polaris TYPE 'rest'
  FOREIGN DATA WRAPPER iceberg_catalog
  OPTIONS (rest_endpoint 'http://polaris:8181',
           client_id '...', client_secret '...',
           location_prefix 's3://my-bucket/warehouse');

CREATE TABLE t (a int) USING iceberg WITH (catalog = 'my_polaris');

A server with no options is also valid — all settings fall back to GUCs. This allows creating a named handle for organizational purposes while relying entirely on the global GUC configuration.

Extension Upgrade Script

The upgrade script creates:

iceberg_catalog FDW — a handler-less FDW with a validator function (iceberg_catalog_validator) that accepts server-level options.
GRANT USAGE ON FOREIGN DATA WRAPPER iceberg_catalog TO lake_write — so non-superusers with lake_write can create catalog servers.

The reserved catalog names postgres, object_store, and rest are not created as foreign servers. They are built-in identifiers whose configuration comes entirely from GUC settings. Only user-created catalogs (via CREATE SERVER ... FOREIGN DATA WRAPPER iceberg_catalog) result in actual foreign server objects.

Connection Resolution

ResolveRestCatalogOptions(catalog) is the single entry point for building a RestCatalogOptions. GetRestCatalogOptionsForRelation(relationId) reads the table's catalog option and delegates to it.

The resolution path is split into two builders plus a dispatcher:

ResolveRestCatalogOptions(catalog) — if catalog matches the built-in rest name (case-insensitive via pg_strcasecmp), call BuildRestCatalogOptionsFromGUCs(); otherwise call BuildRestCatalogOptionsFromServer(catalog).
BuildRestCatalogOptionsFromGUCs() — materializes options entirely from GUCs. Sets opts->catalog to the canonical REST_CATALOG_NAME constant ("rest").
BuildRestCatalogOptionsFromServer(serverName) — looks up the server in pg_foreign_server, initializes all fields from GUC defaults via ApplyGUCDefaults, then overrides with any options explicitly set on the server via ApplyServerOptionOverrides (driven by iceberg_catalog_option_descs[]).
ValidateRestCatalogOptions — errors if rest_endpoint is still unset after both sources.

catalog_name on the server is stored in opts->catalogName when present, but it is not defaulted to get_database_name() inside resolution. For read-only tables, the database-name default is applied at CREATE TABLE time: if the table does not specify catalog_name, create_table.c inherits the server's catalog_name option (if set) or defaults to get_database_name(MyDatabaseId), then bakes the result into the table's ftoptions. Writable tables ignore server catalog_name at runtime (see below).

Per-Catalog Token Cache

The old code used a single global token (RestCatalogAccessToken). With multiple catalogs potentially using different credentials, the token cache is now a hash table (RestCatalogTokenCache) keyed by catalog name. Each catalog gets its own cached OAuth token with independent expiry tracking.

The hash table and token strings are allocated in a dedicated RestTokenCacheCtx memory context (under CacheMemoryContext), keeping the cache memory isolated. BuildTokenCacheKey uses opts->catalog as the cache key (zeroed to TOKEN_CACHE_KEY_LEN to avoid hash collisions on short names).

Invalidation on server DDL: a syscache invalidation callback (InvalidateRestTokenCache) is registered once per backend via CacheRegisterSyscacheCallback(FOREIGNSERVEROID, ...). Any ALTER SERVER or DROP SERVER resets the entire token cache (MemoryContextReset(RestTokenCacheCtx)) so stale credentials are never reused in open sessions. The cache is rebuilt lazily on the next token lookup. TokenCacheCallbackRegistered ensures the callback is registered exactly once (the syscache callback array is fixed-size).

SendRequestToRestCatalog implements its own retry loop (up to 3 retries) with a ClassifyRestCatalogRequestRetry classifier that determines whether to back off, refresh auth, or stop. When a 419 (token expired) response is received, the retry logic force-refreshes the token via GetRestCatalogAccessToken(opts, true) and patches the Authorization header in-place via UpdateAuthorizationHeader. The opts parameter is passed directly to SendRequestToRestCatalog, avoiding any global state. Pass opts = NULL for the token-fetch request itself to avoid recursion.

Single-Catalog Transaction Constraint

Modifying tables from different REST catalogs in the same transaction is rejected at statement time by BindRelationToXactRestCatalog(relationId), called from both FDW write entry points:

postgresBeginForeignModify() — row-by-row DML
AddQueryResultToTable() — INSERT...SELECT and COPY...FROM pushdown

All per-transaction REST catalog state is grouped in a single PgLakeXactRestCatalogContext struct (static PgLakeXactRestCatalog), which contains:

requestsHash — hash table of per-table REST catalog requests
commitContext — pre-allocated 1MB memory context for XACT_COMMIT
catalogOpts — deep-copied RestCatalogOptions for the single catalog

On the first REST-backed write of the transaction, BindRelationToXactRestCatalog pre-resolves the relation's full RestCatalogOptions and deep-copies them into TopTransactionContext (PgLakeXactRestCatalog->catalogOpts). This happens before any Parquet data is written to S3. Subsequent writes in the same transaction must target the same catalog (compared with pg_strcasecmp); otherwise ERRCODE_FEATURE_NOT_SUPPORTED is raised immediately.

DDL paths (CREATE TABLE / DROP TABLE) reach the same protection indirectly via RecordRestCatalogRequestInTx(), which is invoked synchronously at statement time from the utility hook. That function retains a belt-and-suspenders cross-catalog check for any code path that forgets to bind at statement time.

At XACT_EVENT_COMMIT time, PostAllRestCatalogRequests uses the pre-resolved PgLakeXactRestCatalog->catalogOpts to build URLs and authentication headers. This avoids syscache lookups during commit, which are forbidden (AssertCouldGetRelation fires during TRANS_COMMIT state).

When an iceberg table is created on a user-defined server, RecordIcebergCatalogServerDependency records a DEPENDENCY_NORMAL edge from the table to the server in pg_depend. This ensures DROP SERVER is blocked while dependent tables exist (and DROP SERVER CASCADE drops them). Built-in catalog names (rest, postgres, object_store) do not get a dependency entry because they are not backed by a user-managed pg_foreign_server row.

DDL Protection (`ValidateIcebergCatalogServerDDL`)

A single ProcessUtility hook validates all DDL on iceberg_catalog servers. The handler is registered in pg_lake_iceberg/src/init.c (where the function is defined), not in pg_lake_table.

Operation	Effect
`CREATE SERVER` with reserved name (`postgres`, `object_store`, `rest`, case-insensitive)	Blocked
`CREATE SERVER` with TYPE `'postgres'` or `'object_store'`	Blocked
`CREATE SERVER` without TYPE `'rest'` (NULL or non-rest)	Blocked
`CREATE SERVER` with TYPE `'rest'`	Allowed
`ALTER SERVER ... RENAME TO` any reserved name	Blocked
`ALTER SERVER ... RENAME TO` any non-reserved name on an `iceberg_catalog` server	Blocked (dependent tables store `catalog='<name>'` in `ftoptions`)
`ALTER SERVER OPTIONS (SET/ADD rest_endpoint ...)` when dependent REST iceberg tables exist	Blocked (writable and read-only, via `ServerHasDependentRestIcebergTable`)
`ALTER`/`DROP`/`OWNER` on reserved names	Fail naturally (no server object exists)
All other DDL on user-created servers	Allowed

All protection checks are skipped during CREATE EXTENSION / ALTER EXTENSION.

CREATE FOREIGN TABLE on any iceberg_catalog server is also blocked since the FDW has no handler. The error hints users to use CREATE TABLE ... USING iceberg instead.

Catalog Type Helpers (`pg_lake_engine/src/utils/catalog_type.c`)

IsRestCatalog(catalog) — unified check that returns true for the literal 'rest' (case-insensitive) or any iceberg_catalog server whose TYPE is 'rest'. Asserts that the server type is non-null and non-empty when a server object exists. Used by the GUC check hook for default_catalog to accept user-created server names.
IsCatalogOwnedByExtension(catalog) — returns true for the literals 'rest', 'object_store', or 'postgres' (all case-insensitive via pg_strcasecmp). Used by the DDL protection handler and dependency recording.

Option Validation

Server options are defined once in iceberg_catalog_option_descs[] — the single source of truth for the whitelist (FindCatalogOptionDesc), the user-facing hint (GetValidCatalogOptionsHint), and the option-to-struct applier (ApplyCatalogOptionValue). Adding or removing an option requires changing only this table.

FDW option names and auth type values are compared case-insensitively (pg_strcasecmp). Server names remain case-sensitive (PostgreSQL's GetForeignServerByName is case-sensitive), but the reserved built-in names are checked case-insensitively.

At CREATE/ALTER SERVER time, ValidateCatalogOptionValue enforces:

rest_endpoint, oauth_endpoint, location_prefix: non-empty and must contain :// (URI scheme required)
catalog_name, client_id, client_secret, scope: non-empty when present
rest_auth_type: must be 'oauth2', 'default' (alias for oauth2), or 'horizon'
enable_vended_credentials: valid boolean

The validator hint string is built lazily from iceberg_catalog_option_descs[] and cached in TopMemoryContext so it survives transaction aborts on the error path (a per-statement context would leave a dangling pointer on the second invalid-option failure in the same backend).

`catalog_name` Server Option

The catalog_name option on an iceberg_catalog server specifies the catalog prefix used in REST API URLs (opts->catalogName).

At CREATE TABLE time (read-only tables): if the table does not specify catalog_name, it inherits the server's catalog_name option (via ResolveRestCatalogOptions) or defaults to get_database_name(MyDatabaseId). The resolved value is baked into the table's ftoptions.

At runtime (GetRestCatalogName):

Writable tables always return get_database_name(MyDatabaseId). Server and table catalog_name options are ignored. This prevents a subsequent ALTER SERVER ... ADD/SET catalog_name from silently re-routing an existing writable table.
Read-only tables return the table-level catalog_name from ftoptions. If missing, error (should never happen after CREATE TABLE inheritance).

Writable tables cannot be created on a server with catalog_name set — enforced at CREATE TABLE time.

Accepted Server Options

The iceberg_catalog_validator accepts the following server-level options (all defined in iceberg_catalog_option_descs[]):

Option	Description
`rest_endpoint`	REST catalog base URL (required; non-empty, must have URI scheme)
`rest_auth_type`	`'oauth2'` (default), `'default'` (alias for oauth2), or `'horizon'`
`oauth_endpoint`	Custom OAuth token endpoint
`scope`	OAuth scope (default from GUC: `PRINCIPAL_ROLE:ALL`)
`enable_vended_credentials`	Boolean, default `true`
`location_prefix`	S3/storage prefix, overrides the GUC default
`catalog_name`	Catalog name passed to REST API (affects read-only table CREATE TABLE; ignored at runtime for writable tables)
`client_id`	OAuth client ID
`client_secret`	OAuth client secret

Credentials are currently stored as server options. USER MAPPING support is planned in a follow-up PR.

Backward Compatibility

CREATE TABLE ... WITH (catalog = 'rest') continues to work. The catalog option value 'rest' resolves to the built-in configuration sourced entirely from GUCs (case-insensitive: 'REST', 'rEst', etc. all work).
All existing GUCs remain functional and serve as fallback defaults.
No changes to postgres or object_store catalog behavior.

Fixes part of #230

Checklist

Probably need to intercept create server commands that use variations of case-insensitive "postgres", "rest" and "object_store" reserved literals.
I have tested my changes and added tests if necessary
I updated documentation if needed
I confirm that all my commits are signed off (DCO)

sfc-gh-abozkurt · 2026-03-05T10:43:15Z

 extern PGDLLEXPORT bool HasRestCatalogTableOption(List *options);
 extern PGDLLEXPORT bool HasObjectStoreCatalogTableOption(List *options);
 extern PGDLLEXPORT bool HasReadOnlyOption(List *options);
+extern PGDLLEXPORT bool IsServerBasedRestCatalog(List *options);


that sounds a bit weird. maybe IsRestCatalogOwnedByExtension and IsRestCatalogOwnedByUsers

First I renamed to IsRestCatalogOwnedByUsers but then I realized we don't really need the distinction so I changed it to IsRestCatalog. Details in the last commit of this PR

sfc-gh-abozkurt · 2026-03-05T10:45:45Z

+	if (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)


maybe IsCatalogOwnedByExtension, that catches all our catalogs

sfc-gh-abozkurt · 2026-03-05T10:46:22Z

+	if (server->servertype != NULL && *server->servertype != '\0')
+		return pg_strncasecmp(server->servertype, "rest", strlen("rest")) == 0;
+
+	/* No TYPE specified, assume rest */


should we disallow that?

we can allow since 'rest' is the only type that is allowed for users for now

+1 for disallowing that, perhaps on create server, and we can assert here that TYPE cannot be NULL or empty.

sfc-gh-abozkurt · 2026-03-05T10:47:38Z

-static char *RestCatalogAccessToken = NULL;
-static TimestampTz RestCatalogAccessTokenExpiry = 0;
+ * Per-server token cache. Keyed by server name (for server-based catalogs)
+ * or "GUC" (for GUC-based backward-compatible catalog='rest').


instead of GUC, cant we still have server name for our catalogs?

makes sense

sfc-gh-abozkurt · 2026-03-05T10:50:58Z

+			conn->host = defGetString(def);
+		else if (strcmp(def->defname, "client_id") == 0)
+			conn->clientId = defGetString(def);
+		else if (strcmp(def->defname, "client_secret") == 0)


is this user mapping or server option?

User mapping. Here only for the sake of separate server infrastructure.

sfc-gh-abozkurt · 2026-03-05T11:00:25Z

+			if (pg_strncasecmp(catalogOptionValue, REST_CATALOG_NAME, strlen(catalogOptionValue)) == 0)
+				conn = GetRestCatalogConnectionFromGUCs();
+			else
+				conn = GetRestCatalogConnectionFromServer(catalogOptionValue);


a wrapper seems useful

Since both extension-owned and user-created REST type catalog servers fallback to GUC, there is no need for a distinction here with if-else, as I changed in my last commit.

sfc-gh-abozkurt · 2026-03-05T11:01:07Z


 			metadataLocation =
-				GetMetadataLocationFromRestCatalog(catalogName, catalogNamespace, catalogTableName);
+				GetMetadataLocationFromRestCatalog(conn, catalogName, catalogNamespace, catalogTableName);


we might fetch conn inside GetMetadataLocationFromRestCatalog and do not change its signature maybe.

We would either need the relation ID or the server name to fetch conn from the inside, so signature change is needed.

sfc-gh-abozkurt · 2026-03-05T11:02:38Z

 	{
 		if (!requestPerTable->isValid)
 		{
-			/*


I guess this comment should stay.

sfc-gh-abozkurt · 2026-03-05T11:02:50Z

 			requestPerTable->dropTableRequest != NULL)
-		{
-			/*
-			 * table is created and dropped in the same transaction, nothing


same, comment should stay

sfc-gh-abozkurt · 2026-03-05T11:03:59Z

 				HttpResult	httpResult =
 					SendRequestToRestCatalog(HTTP_POST, requestPerTable->tableRestUrl,
-											 createTableRequest->body, PostHeadersWithAuth());
+											 createTableRequest->body, PostHeadersWithAuth(requestPerTable->conn));


we need to add tests where we modify multiple rest tables inside the same transaction to cover the changes.

sfc-gh-abozkurt

Good job! It seems very close to merge to me.

sfc-gh-abozkurt · 2026-03-13T07:54:54Z

+extern PGDLLEXPORT RestCatalogConnectionInfo * GetRestCatalogConnectionForRelation(Oid relationId);
+
+/* FDW name for iceberg_catalog servers */
+#define ICEBERG_CATALOG_FDW_NAME "iceberg_catalog"


nit: this can be a more generic place

sfc-gh-abozkurt · 2026-03-13T07:57:54Z

+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("invalid rest_auth_type option: \"%s\"", authType),
+						 errhint("Valid values are \"default\" and \"horizon\".")));


nit: does it makes sense to name default (oauth2 or such)?

sfc-gh-abozkurt · 2026-03-13T07:59:38Z

+ *  - ALTER ... RENAME on 'postgres', 'object_store', or 'rest' is blocked.
+ */
+bool
+ProtectExtensionCatalogServersHandler(ProcessUtilityParams *processUtilityParams,


maybe BlockDdlOnExtensionCatalogs

sfc-gh-abozkurt · 2026-03-13T08:04:11Z

+	}
+	else if (IsA(parsetree, AlterForeignServerStmt))
+	{
+		AlterForeignServerStmt *stmt = (AlterForeignServerStmt *) parsetree;


Can you add a test tho show ALTER SERVER OWNER TO is also disabled?

Good point, I may need to intercept AlterOwnerStmt for that

sfc-gh-abozkurt · 2026-03-13T08:06:30Z

+	ForeignServer *server = GetForeignServerByName(serverName, false);
+	ForeignDataWrapper *fdw = GetForeignDataWrapper(server->fdwid);
+
+	if (strcmp(fdw->fdwname, ICEBERG_CATALOG_FDW_NAME) != 0)


should we prevent that at CREATE FOREIGN SERVER time by hooking into it? Here we can have assertion.

Good catch - actually by this point the server is guaranteed to be iceberg_catalog, so just an Assert is sufficient.

sfc-gh-abozkurt · 2026-03-13T08:15:16Z

-		if (!RestCatalogClientId || !*RestCatalogClientId)
-			ereport(ERROR, (errmsg("pg_lake_iceberg.rest_catalog_client_id should be set")));
+		if (!conn->clientId || !*conn->clientId)
+			ereport(ERROR, (errmsg("REST catalog client_id is not configured")));


hint could be alter server option or GUC, same for others for which it applies

sfc-gh-abozkurt · 2026-03-13T08:16:39Z

+		 * Invalidate all cached tokens so that the next request will fetch a
+		 * fresh one. We clear the entire cache because the retry callback
+		 * does not have access to a specific connection's info. This is safe
+		 * because token expiry is rare and other connections will simply
+		 * re-authenticate on their next request.


shouldnt we invalidate only one token that belongs to the catalog which we send request to? i.e. HASH_REMOVE

Good point, we could pass the servername to SendRequestToRestCatalog

sfc-gh-abozkurt · 2026-03-13T08:18:53Z


 	AvroInit();
+
+	RegisterUtilityStatementHandler(ProtectExtensionCatalogServersHandler, NULL);


hooks should exist in pg_lake_table extension. pg_lake_iceberg is only used for iceberg spec and catalog implementations and iceberg specific internal tables or views.

sfc-gh-abozkurt · 2026-03-13T08:32:20Z

+	 */
+	while (list_length(tablesWithModifications) > 0)
+	{
+		RestCatalogRequestPerTable *firstTable =


Just asking. Cant we still send a single request? At the first sight, I see polaris open api seems to not allow it. @sfc-gh-okalaci

You can't send a single HTTP request for multiple hosts. The code currently creates batches based on host.

But, now that you mention it, what if we have two different catalogs, from the same host? This could be a bug. Maybe we should batch on (host, catalog_name) and not simply on (host)

sfc-gh-abozkurt · 2026-03-13T08:35:49Z

+    assert result[0]["srvtype"] == "postgres"
+
+
+def test_precreated_object_store_server(pg_conn, extension):


precreated_rest?

sfc-gh-abozkurt · 2026-03-13T08:55:28Z

I think we should disable table creation with iceberg_catalog fdw as it has no handler via hooks. (we already have create table hooks)

postgres=# create foreign table test (a timestamptz) server postgres;
CREATE FOREIGN TABLE
postgres=# table test;
ERROR:  foreign-data wrapper "iceberg_catalog" has no handler

sfc-gh-okalaci · 2026-03-18T11:51:45Z

 			{
 				HttpResult	httpResult =
 					SendRequestToRestCatalog(HTTP_POST, requestPerTable->tableRestUrl,
-											 createTableRequest->body, PostHeadersWithAuth());


I think we should undo most of the changes here. It is extremely hard to support multiple rest catalogs involved in a tx, especially with modifications involved.

We currently do not even have a mechanism to handle failures if the request for a single REST catalog fails.

When we have multiple REST catalogs involved, the failure handling / recovery becomes extremely hard to handle. What happens if request to server A succeeds and request to server B fails?

I think if we merge this PR as-is, we'd make it much harder to handle the recovery scenarios, which we have not even handled even for a single end-point.

So, let's just fail the transaction pre-commit XACT_EVENT_PRE_COMMIT, perhaps in RecordRestCatalogRequestInTx where we throw error if you ever try to record changes to more than one rest catalog.

In general, many query engines do not even support multi-table transactions yet along multi-server :) So, we are not contradicting with any user expectations.

Good point, I overlooked this part.

sfc-gh-okalaci · 2026-03-18T11:58:58Z

+{
+	for (int i = 0; iceberg_catalog_server_options[i] != NULL; i++)
+	{
+		if (strcmp(keyword, iceberg_catalog_server_options[i]) == 0)


should this be case insensitive? It is more common in pg_lake and in Postgres to allow case insensitive keywords.

Make sure it works in other places as well if we change it, for example some tests should use different cases

sfc-gh-okalaci · 2026-03-18T11:59:48Z

+			char	   *authType = defGetString(def);
+
+			if (strcmp(authType, "oauth2") != 0 &&
+				strcmp(authType, "default") != 0 &&


similarly: can we allow case insensitive values? make sure if we allow it here, we check in other places similarly

sfc-gh-okalaci · 2026-03-18T12:10:16Z

 				 URLEncodePath(catalogName));

-	HttpResult	httpResult = SendRequestToRestCatalog(HTTP_POST, postUrl, body.data, PostHeadersWithAuth());
+	HttpResult	httpResult = SendRequestToRestCatalog(HTTP_POST, postUrl, body.data,


nit: I think we have some indentation issues in the PR, such as this one. Can you maybe go over once more regarding that? Do you use make reindent? That seems to fix some of these on my local

sfc-gh-okalaci · 2026-03-18T12:16:20Z

 	const int	MAX_HTTP_RETRY_FOR_REST_CATALOG = 3;

-	return SendHttpRequestWithRetry(method, url, body, headers, ShouldRetryRequestToRestCatalog, MAX_HTTP_RETRY_FOR_REST_CATALOG);
+	CurrentRetryServerName = serverName;


static variables are always painful to maintain, what if SendHttpRequestWithRetry throws an error? We are left with the old server name. I'm pretty sure that cause hard-to-understand issues/consequences.

I think what we are trying here is to pass ShouldRetryRequestToRestCatalog the server name.

Can't we change HttpRetryFn signature to pass the server name?

sfc-gh-okalaci · 2026-03-18T12:17:43Z

+  VALIDATOR lake_iceberg.iceberg_catalog_validator;
+
+/* Pre-created catalog servers for backward compatibility */
+CREATE SERVER postgres


uf, the name postgres might be used by the users, say for an postgres_fdw server.

I wonder if we should pick something harder to hit? Can you think of anything prefix/postfix for the pre-created servers?

sfc-gh-okalaci · 2026-03-18T12:19:45Z


-		GetRestCatalogAccessToken(forceRefreshToken);
+			strlcpy(cacheKey, CurrentRetryServerName, TOKEN_CACHE_KEY_LEN);
+			hash_search(RestCatalogTokenCache, cacheKey, HASH_REMOVE, NULL);


hm, we remove the token from the cache, and return true, so the caller would simply re-use the existing headers to retry, which means will use the old token.

I think we should re-generate token and add here? Especially given we return true, we should do something like that.

Good point, we can use GetRestCatalogAccessToken like the previous code was using. We wouldn't even need hash_search(..., HASH_REMOVE, ...) since there is existing forceRefreshToken logic.
Note that Claude is suggesting we had a previous bug of using the same pre-built headers after GetRestCatalogAccessToken(forceRefreshToken); So taking care of that as well.

Opened an issue for this: #276
Not sure if we should solve it in this PR or separately

EDIT: maybe better to solve this separately as this PR is getting huge #278

sfc-gh-okalaci · 2026-03-18T12:23:13Z

+	"rest_auth_type",
+	"oauth_endpoint",
+	"enable_vended_credentials",
+	"location_prefix",


we dont seem to be using location_prefix ? I mean, even if I provide that, we still use GetIcebergDefaultLocationPrefix when needed in rest_catalog.c?

I think if we support this, we should have tests such that we set GetIcebergDefaultLocationPrefix to return a location that's not working, create/modify/vacuum/ddl tables such that make sure we don't accidentally use it.

Sorry about that, huge miss from my part.

nit: I am reusing the "remove trailing slash" code - think I'll open a separate PR about that since we are also using it for stage_location and other things.

EDIT: opened and merged separate PR on this #275

sfc-gh-okalaci · 2026-03-18T12:25:47Z

 {
 	char	   *newCatalog = *newvalue;

-	if (pg_strncasecmp(newCatalog, POSTGRES_CATALOG_NAME, strlen(newCatalog)) == 0 ||


aren't we supposed to allow use-defined catalogs?

I thought one of the most important pieces of this PR is to let users easily use/switch between the servers, and this seems like a good place to be able to do that?

Needs tests to show we allow user created rest server, but not allow say postgres_fdw servers or unrelated/non-existing ones

Good catch! I forgot about this part.

sfc-gh-okalaci · 2026-03-18T12:32:56Z

+		if (!IsIcebergCatalogServer(stmt->servername))
+			return false;
+
+		if (pg_strcasecmp(stmt->servername, POSTGRES_CATALOG_NAME) == 0 ||


I think postgres server name is case sensitive? https://github.com/postgres/postgres/blob/4f433025f666fa4a6209f0e847715767fb1c7ace/src/backend/foreign/foreign.c#L794

I think for server names we should use strcmp ?

ps: good idea to test two servers: CREATE SERVER "test" and CREATE SERVER "TEST" in the same db, with different options and make sure they both work fine?

Could do that.
In that case, we would allow names like "Object_Store", so functions like HasObjectStoreCatalogTableOption should also be case sensitive.

on second thought, I think we should keep case insensitive comparisons against POSTGRES_CATALOG_NAME, OBJECT_STORE_CATALOG_NAME and REST_CATALOG_NAME. We can't have "rest" and "REST". But we can have "test" and "TEST".

sfc-gh-okalaci · 2026-03-18T12:35:25Z

+
+/*
+* Gets an access token from rest catalog. Caches the token per server
+* (keyed by host + clientId) until it is about to expire.


I think this comment keyed by host + clientId) is stale?

true, it's keyed by server name

sfc-gh-okalaci · 2026-03-18T12:37:09Z

+			requestPerTable = (RestCatalogRequestPerTable *) lfirst(lc);

-			if (!lastRequest)
+			if (strcmp(requestPerTable->conn->host, batchHost) != 0 ||


we'll remove this code, but anyway this is not safe, we can have multiple REST catalogs in the same host? So we cannot really compare the host?

If we need this kind of grouping in any other place in the code, we should do with the OID of the server instead?

It is actually checking both host and catalog name which is unique.

if (strcmp(requestPerTable->conn->host, batchHost) != 0 || strcmp(requestPerTable->catalogName, catalogName) != 0)

Server name/oid would also work (better since it would be a single check).

sfc-gh-okalaci · 2026-03-18T12:39:14Z


 	MarkGUCPrefixReserved(PG_LAKE_TABLE);

+	RegisterUtilityStatementHandler(BlockDDLOnExtensionCatalogs, NULL);


should we register this in pg_lake_icebeg? Any reason to do it in pg_lake_table?

#242 (comment)

…pport Introduce the iceberg_catalog foreign data wrapper as a configuration framework for Iceberg catalogs. This allows defining named catalog servers via CREATE SERVER, removing the limitation of a single global REST catalog configured through GUCs. Key changes: - Add iceberg_catalog_validator for server option validation - Add RestCatalogConnectionInfo struct to unify connection parameters - Add GetRestCatalogConnectionFromGUCs/FromServer resolution functions - Refactor all REST catalog functions to accept RestCatalogConnectionInfo - Refactor token cache from single global to per-server hash table - Add extension upgrade SQL (3.2--3.3) with FDW, validator, and pre-created postgres/object_store servers Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Update IsServerBasedRestCatalog and HasRestCatalogTableOption to check whether a catalog option value refers to an iceberg_catalog foreign server. Servers without an explicit TYPE default to rest. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Update pg_lake_iceberg_validator to accept server names as valid catalog option values (in addition to the existing postgres/object_store/rest literals). Update ProcessCreateIcebergTableFromForeignTableStmt to resolve REST catalog connection info from the named server when the catalog option refers to an iceberg_catalog server. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Refactor transaction-level REST catalog request tracking to resolve and store a RestCatalogConnectionInfo per table. Batch commit requests are now grouped by REST catalog host, so tables using different catalog servers within the same transaction are committed to the correct endpoints independently. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Comprehensive test suite covering: - FDW existence and pre-created postgres/object_store servers - CREATE SERVER with valid and invalid options - Foreign table creation on handler-less FDW (query-time error) - ALTER/DROP SERVER operations - Server-based catalog references in CREATE TABLE - Backward compatibility with catalog='rest'/'postgres'/'object_store' Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

1 - Add extension owned rest catalog of type 'rest' 2 - Disallow creation of further 'postgres' or 'object_store' type catalogs unless we are creating_extension 3 - Disallow renaming/dropping extension owned catalogs. 4 - Disallow altering the options of extension-owned postgres and object_store catalogs. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

- Add helper functions: - IsRestCatalogOwnedByExtension ('rest' name) - IsCatalogOwnedByExtension ('postgres', 'object_store', 'rest') - Rename IsServerBasedRestCatalog to IsRestCatalogOwnedByUsers - Use servername per server token cache - Add tests where we modify tables from different catalogs(rest_endpoints) in the same transaction. - Keep some comments I accidentally removed. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

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>

- Block CREATE FOREIGN TABLE on iceberg_catalog servers. The iceberg_catalog FDW has no handler, so foreign tables created on it would fail at query time with "has no handler". The check is added to ErrorUnsupportedCreatePgLakeTableHandler in pg_lake_table, which already runs first for all CREATE FOREIGN TABLE statements. - Block ALTER SERVER ... OWNER TO on extension-owned catalog servers (postgres, object_store, rest). - Move ICEBERG_CATALOG_FDW_NAME from rest_catalog.h to catalog_type.h alongside the other catalog name constants, since it is referenced by both pg_lake_iceberg and pg_lake_table. - Rename rest_auth_type value "default" to "oauth2" to better describe the standard OAuth2 client_credentials grant with Basic auth. "default" value is also kept. - Rename ProtectExtensionCatalogServersHandler to BlockDDLOnExtensionCatalogs for clarity. - Move BlockDDLOnExtensionCatalogs registration from pg_lake_iceberg init to pg_lake_table init, where all other ProcessUtility hooks are registered. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Token cache improvements: - Allocate the token cache hash table and token strings in a dedicated RestTokenCacheCtx memory context (under TopMemoryContext) instead of directly in TopMemoryContext, keeping the cache memory isolated. - On a 419 (token expired) retry, invalidate only the affected server cached token instead of clearing the entire cache. The server name is passed through SendRequestToRestCatalog and stored in a file-scoped static so the retry callback can target the right entry. Transaction commit batching fix: - Group batches by (host, catalogName) instead of host alone. The transaction commit URL includes the catalog prefix, so two servers pointing to the same host but with different catalog_name values need separate commits. Previously, all tables on the same host were batched together using only the first table catalogName. Other improvements: - Replace the ereport(ERROR) FDW name check in GetRestCatalogConnectionFromServer with an Assert, since the catalog option validator already ensures only iceberg_catalog servers are accepted. - Add errhint to credential/host error messages in FetchRestCatalogAccessToken, pointing users to both the server option and the corresponding GUC. - Add test_precreated_rest_server test. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Block CREATE SERVER with a name that case-insensitively matches the extension-owned catalog names (postgres, object_store, rest) on the iceberg_catalog FDW. Also block ALTER SERVER ... RENAME TO a reserved name. Fix a latent bug across multiple files where pg_strncasecmp was used with a partial length, causing prefix-only matching: e.g. "rest_1" would match "rest". Replaced all instances with pg_strcasecmp for exact case-insensitive comparison. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

REST catalog options and retry mechanism: - Rename RestCatalogConnectionInfo to RestCatalogOptions throughout - Eliminate CurrentRetryServerName static; pass opts directly through HttpRetryFn callback (void *context + List *headers) so the 419 token-expired handler can force-refresh and patch the Authorization header in-place - Fix double-free in GetRestCatalogAccessToken: null out entry fields before calling FetchRestCatalogAccessToken Transaction-scoped state: - Reject transactions that touch tables from different REST catalog servers - Replace per-table rest catalog opts deep-copy with a single PgLakeXactRestCatalogOpts static, deep-copied into TopTransactionContext on first use - This avoids syscache lookups at XACT_EVENT_COMMIT time, which are forbidden (AssertCouldGetRelation fires during TRANS_COMMIT state) Server configuration enforcement: - Require TYPE 'rest' on CREATE SERVER ... FOREIGN DATA WRAPPER iceberg_catalog (reject NULL or non-rest types) - Make FDW option names and auth type values case-insensitive (pg_strcasecmp), while keeping server names case-sensitive - Make reserved catalog name checks (postgres, object_store, rest) case-insensitive via IsCatalogOwnedByExtension - Support location_prefix server option, overriding the GUC default - Accept user-created iceberg_catalog servers in default_catalog GUC Tests: - Add test_reject_modify_different_rest_catalogs_in_single_transaction - Add test_server_location_prefix_overrides_guc - Add tests for TYPE enforcement, case-sensitive server names, and default_catalog GUC with user-created servers - Remove obsolete no-TYPE-defaults-to-rest tests Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Don't create foreign servers for the reserved catalog names in the extension install script. These names ('postgres', 'object_store', 'rest') could already be in use in the target database (especially 'postgres'), causing the extension script to fail. Instead, treat them as special built-in identifiers whose configuration comes entirely from GUC settings. Only user-created catalogs (via CREATE SERVER ... FOREIGN DATA WRAPPER iceberg_catalog) result in actual foreign server objects. Key changes: - Remove CREATE SERVER postgres/object_store/rest from the upgrade SQL. - GetRestCatalogOptionsFromCatalog now returns GUC-only opts for the built-in 'rest' name without looking up a foreign server. - Simplify BlockDDLOnExtensionCatalogs: only guard CREATE SERVER (reserved names, TYPE postgres/object_store) and RENAME TO reserved names. ALTER/DROP/OWNER on built-in names fail naturally since no server object exists. - Remove IsIcebergCatalogServer (no longer needed). - Rename RestCatalogOptions.serverName to .catalog and GetRestCatalogOptionsFromServer to GetRestCatalogOptionsFromCatalog to reflect the new semantics. - Update tests Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

The catalog_name option was accepted on iceberg_catalog servers but never consumed. Now GetRestCatalogOptionsFromCatalog reads it and stores it in RestCatalogOptions.catalogName. GetRestCatalogName uses a three-level fallback for the REST API catalog prefix: table option > server option > database name. This lets a server define a default catalog_name for all its tables while still allowing per-table overrides. - Add catalogName field to RestCatalogOptions. - Populate it from the server catalog_name option in GetRestCatalogOptionsFromCatalog. - Rewrite GetRestCatalogName to check table option first, then opts->catalogName. - Writable REST catalog tables cannot use a server with catalog_name set, since for now they inherit from the database name, schema name, and table name - Add tests Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

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

Consolidate the three per-transaction REST catalog statics (RestCatalogRequestsHash, PgLakeXactCommitContext, PgLakeXactRestCatalogOpts) into a single RestCatalogXactState struct to reduce global state and make lifetime management clearer. Merge BlockDDLOnExtensionCatalogs and RequireRestTypeForIcebergCatalogServer into a single ValidateIcebergCatalogServerDDL hook, eliminating duplicate hook guard logic for CREATE SERVER validation. Grant USAGE on iceberg_catalog FDW to lake_write so non-superusers with write permissions can create catalog servers. Additional cleanups: - Convert unreachable host check in FetchRestCatalogAccessToken to Assert - Rename FDW validator parameter from 'catalog' to 'catalogRelId' - Replace two separate override tests with a single parameterized test_server_option_overrides_guc covering rest_endpoint, client_id, client_secret, location_prefix, and catalog_name - Add tests for no-option server creation and lake_write permissions Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

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

Harden catalog server security, configuration, and cross-catalog safety Refactor GetRestCatalogOptionsFromCatalog into a clean dispatcher (ResolveRestCatalogOptions) with two explicit builders: BuildRestCatalogOptionsFromGUCs for the built-in 'rest' path and BuildRestCatalogOptionsFromServer for user-created servers, plus shared helpers ApplyGUCDefaults, ApplyServerOptionOverrides, and ValidateRestCatalogOptions. Add CopyRestCatalogOptions for safe deep-copy into a target memory context. Unify the three separate server option lists (whitelist array, errhint string, applier chain) into a single IcebergCatalogOptionDesc descriptor table with type, offsetof, and validation flags. Adding or removing an option is now a one-line change. Add DDL-time value validation: reject empty strings for client_id, client_secret, scope, catalog_name; require a URI scheme for rest_endpoint, oauth_endpoint, location_prefix. Require ACL_USAGE on user-created iceberg_catalog servers at CREATE TABLE time, matching core's CREATE FOREIGN TABLE semantics. Record a DEPENDENCY_NORMAL from iceberg tables to their catalog server in pg_depend so DROP SERVER is blocked (and CASCADE drops them). Block ALTER SERVER RENAME for iceberg_catalog servers since dependent tables store the server name as a string in ftoptions. Block ALTER SERVER SET rest_endpoint when dependent writable tables exist to prevent silently redirecting them to a different REST catalog. Make GetRestCatalogName always return get_database_name(MyDatabaseId) for writable tables so ALTER SERVER ADD catalog_name cannot re-route an existing table to a different namespace. Fix token cache hash key regression: zero the key buffer with MemSet before strlcpy in BuildTokenCacheKey. Add syscache invalidation callback on FOREIGNSERVEROID to reset the token cache on ALTER/DROP SERVER, using CacheMemoryContext as parent. Add NULL guard on opts in GetRestCatalogAccessToken. Fix default_catalog GUC check hook to accept values outside a transaction (ALTER SYSTEM + pg_reload_conf path), mirroring how PostgreSQL handles check_default_tablespace. Introduce ValidateXactRestCatalog as a fail-fast guard that checks cross-catalog DML at statement time rather than at XACT_EVENT_PRE_COMMIT. Planted in postgresBeginForeignModify and AddQueryResultToTable. The existing pre-commit check is retained as a belt-and-suspenders fallback. Parametrize test_writable_rest_iceberg_table over built-in 'rest' and user-created server paths. Add tests for USAGE enforcement, dependency tracking, server rename blocking, rest_endpoint blocking, catalog_name re-routing, token cache invalidation, ALTER SYSTEM deferred validation, option value validation, multi-table same-server transactions, and cross-catalog rejection cleanup. Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

…ursor's hands Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

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

Fail fast at statement time on cross-catalog DML. Rename ValidateXactRestCatalog to BindRelationToXactRestCatalog and call it from both FDW write paths (postgresBeginForeignModify and AddQueryResultToTable). The function now binds the transaction to the relation's REST catalog on the first DML and rejects any subsequent statement targeting a different catalog, so the second INSERT errors out before any Parquet is written. The pre-commit hook is kept as a belt-and-suspenders fallback for DDL paths that reach the tracker without going through the new guard. The regression test asserts the second INSERT (not COMMIT) raises. Move ValidateIcebergCatalogServerDDL registration from pg_lake_table to pg_lake_iceberg, where the handler is actually defined. Architecturally this puts ownership of catalog-server DDL validation in the extension that owns the catalog server abstraction. Fix latent dangling-pointer in GetValidCatalogOptionsHint. The static hint cache was allocated via initStringInfo in whatever short-lived context the validator happened to be running in (typically MessageContext), so the second invalid-option failure in the same backend returned freed memory to errhint. Allocate in TopMemoryContext so the cache survives transaction boundaries. The strengthened test_reject_unknown_server_option now issues two failing CREATE SERVER statements on the same connection and asserts the full option list appears in the hint on both attempts; the previous version would not have caught this bug. Also covers earlier review-driven hardening on the same branch: token cache invalidation on ALTER SERVER credentials, ALTER SERVER rest_endpoint blocking for all dependent REST iceberg tables (writable and read-only), and DROP OWNED BY / concurrent DROP SERVER dependency tests. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

The three reserved short catalog names ('postgres', 'object_store', 'rest') previously had no backing pg_foreign_server row, which split the resolution path in two (BuildRestCatalogOptionsFromGUCs for the built-in REST, BuildRestCatalogOptionsFromServer for user-created ones) and prevented us from recording pg_depend edges for tables that use the short names. Pre-create three iceberg_catalog servers at extension upgrade time: pg_lake_postgres_catalog, pg_lake_object_store_catalog, and pg_lake_rest_catalog. The prefixed long names cannot collide with names users may already have, notably the very common 'CREATE SERVER postgres FOREIGN DATA WRAPPER postgres_fdw'. User-facing DDL stays identical: users keep writing WITH (catalog='rest' / 'postgres' / 'object_store'). A new ResolveCatalogServerName helper maps short -> long at server lookup time, and opts->catalog continues to carry the short user-facing name so error messages, the cross-catalog DML guard, and the token cache key never expose the long names. The built-in servers are pure structural anchors: ALTER OPTIONS, ALTER OWNER, RENAME, and DROP are all blocked. Configuration for the built-in REST catalog continues to live in GUCs. The resolution path collapses to a single BuildRestCatalogOptionsFromServer that applies GUC defaults first and then any server options (always empty for the built-ins), so the built-in REST and user-created REST go through one code path. The long names are rejected as user-facing catalog= values by IsRestCatalog, the default_catalog GUC check hook, and create_table.c with a clear hint pointing to the short form. Tables created with the short reserved names now record a pg_depend edge against the corresponding built-in server, so DROP EXTENSION CASCADE cleans them up via the standard dependency walker. ValidateIcebergCatalogServerDDL now also rejects: - CREATE SERVER with one of the internal long names - ALTER SERVER ... RENAME TO one of the internal long names - ALTER SERVER OPTIONS on any built-in server (immutable anchors) - ALTER SERVER ... OWNER TO on any built-in server Two small static helpers (RejectIfBuiltinCatalogServerName and RejectIfModifyingBuiltinCatalogServer) collapse the repeating "is built-in" + ereport patterns into single calls. Upgrade safety: the --3.3--3.4 script does a pre-flight check that errors with a clear hint if any of the three long names already exists in the target database, so ALTER EXTENSION UPDATE fails loudly rather than partway through. Tests: 23 new cases in test_iceberg_catalog_server.py covering the three servers exist and are extension-owned, CREATE / RENAME-TO / ALTER OPTIONS / ALTER OWNER / DROP on the long names all blocked, catalog= with a long name rejected at CREATE TABLE, the pg_depend edge for the built-in postgres catalog, and -- the original collision concern -- a pre-existing postgres_fdw server literally named 'postgres' coexists with pg_lake_postgres_catalog. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

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>

sfc-gh-mslot · 2026-05-27T11:12:01Z

Writable tables always return get_database_name(MyDatabaseId). Server and table catalog_name options are ignored. This prevents a subsequent ALTER SERVER ... ADD/SET catalog_name from silently re-routing an existing writable table.

can this be configurable? We'd likely want to write into a specific catalog name.

sfc-gh-npuka requested a review from sfc-gh-abozkurt March 3, 2026 12:23

sfc-gh-abozkurt reviewed Mar 5, 2026

View reviewed changes

Comment thread pg_lake_iceberg/src/rest_catalog/rest_catalog.c Outdated

sfc-gh-npuka linked an issue Mar 9, 2026 that may be closed by this pull request

New Catalog Configuration via CREATE SERVER #230

Open

8 tasks

sfc-gh-npuka mentioned this pull request Mar 9, 2026

New Catalog Configuration via CREATE SERVER #230

Open

8 tasks

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch 4 times, most recently from 14b402b to ca9adb0 Compare March 12, 2026 11:45

sfc-gh-npuka requested a review from sfc-gh-abozkurt March 12, 2026 12:04

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch from ca9adb0 to c613b0f Compare March 12, 2026 16:13

sfc-gh-abozkurt reviewed Mar 13, 2026

View reviewed changes

sfc-gh-npuka marked this pull request as ready for review March 13, 2026 09:15

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch from da7a92a to 6802bf9 Compare March 13, 2026 11:32

sfc-gh-npuka requested a review from sfc-gh-abozkurt March 13, 2026 20:33

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch from baf1541 to 2c7435f Compare March 14, 2026 16:33

sfc-gh-okalaci reviewed Mar 18, 2026

View reviewed changes

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch 3 times, most recently from 5e2d2a1 to 70891ec Compare April 21, 2026 16:28

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch from 70891ec to 67bd64c Compare May 23, 2026 20:27

sfc-gh-npuka requested a review from sfc-gh-okalaci May 23, 2026 20:36

sfc-gh-npuka and others added 23 commits May 26, 2026 14:10

Cleanup from rebase and fix style

e83fd0b

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

Move changes to pg_lake_iceberg--3.3--3.4.sql

a8236b3

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

Some cleanup from last review addressing since I left everything in C…

6374ee0

…ursor's hands Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>

Fix GetRestCatalogName dead code

46daf15

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

sfc-gh-npuka force-pushed the naisila/catalog_reconfig branch from a0e5ae4 to 87dbbc1 Compare May 26, 2026 11:10


		AvroInit();

		RegisterUtilityStatementHandler(ProtectExtensionCatalogServersHandler, NULL);

		assert result[0]["srvtype"] == "postgres"


		def test_precreated_object_store_server(pg_conn, extension):


		MarkGUCPrefixReserved(PG_LAKE_TABLE);

		RegisterUtilityStatementHandler(BlockDDLOnExtensionCatalogs, NULL);

Conversation

sfc-gh-npuka commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Note to reviewer: Let's assume we can save client secrets as server options for now. Easier to review commit by commit as each commit has its own description. The credentials are part of USER MAPPING in this PR #255

How users can create REST catalogs

Extension Upgrade Script

Connection Resolution

Per-Catalog Token Cache

Single-Catalog Transaction Constraint

DDL Protection (ValidateIcebergCatalogServerDDL)

Catalog Type Helpers (pg_lake_engine/src/utils/catalog_type.c)

Option Validation

catalog_name Server Option

Accepted Server Options

Backward Compatibility

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sfc-gh-npuka Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sfc-gh-abozkurt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

sfc-gh-npuka commented Mar 2, 2026 •

edited

Loading

DDL Protection (`ValidateIcebergCatalogServerDDL`)

Catalog Type Helpers (`pg_lake_engine/src/utils/catalog_type.c`)

`catalog_name` Server Option

sfc-gh-npuka Mar 12, 2026 •

edited

Loading