Skip to content

Commit 0e65cd7

Browse files
Back built-in iceberg catalogs with foreign server objects
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>
1 parent ef097de commit 0e65cd7

6 files changed

Lines changed: 540 additions & 45 deletions

File tree

pg_lake_engine/include/pg_lake/util/catalog_type.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,29 @@
2222

2323
/*
2424
* The allowed values for IcebergDefaultCatalog, case insensitive.
25+
*
26+
* These are the user-facing short names used as the catalog= option value
27+
* on CREATE TABLE ... USING iceberg. Internally they map to the
28+
* prefixed built-in server names below; users never type the prefixed
29+
* names directly.
2530
*/
2631
#define POSTGRES_CATALOG_NAME "postgres"
2732
#define OBJECT_STORE_CATALOG_NAME "object_store"
2833
#define REST_CATALOG_NAME "rest"
2934

35+
/*
36+
* Built-in iceberg_catalog server names. Pre-created by the extension
37+
* upgrade script and exist purely as anchors for pg_depend edges and the
38+
* uniform server-lookup path. All ALTER/DROP/RENAME on these names is
39+
* blocked (configuration for the built-in catalogs lives in GUCs).
40+
*
41+
* The prefix keeps them clear of names users are likely to have already
42+
* used (notably the very common "CREATE SERVER postgres FDW postgres_fdw").
43+
*/
44+
#define PG_LAKE_POSTGRES_CATALOG_SERVER_NAME "pg_lake_postgres_catalog"
45+
#define PG_LAKE_OBJECT_STORE_CATALOG_SERVER_NAME "pg_lake_object_store_catalog"
46+
#define PG_LAKE_REST_CATALOG_SERVER_NAME "pg_lake_rest_catalog"
47+
3048
typedef enum IcebergCatalogType
3149
{
3250
NONE_CATALOG = 0,
@@ -66,3 +84,5 @@ extern PGDLLEXPORT bool HasObjectStoreCatalogTableOption(List *options);
6684
extern PGDLLEXPORT bool HasReadOnlyOption(List *options);
6785
extern PGDLLEXPORT bool IsCatalogOwnedByExtension(const char *catalog);
6886
extern PGDLLEXPORT bool IsRestCatalog(const char *catalog);
87+
extern PGDLLEXPORT const char *ResolveCatalogServerName(const char *catalog);
88+
extern PGDLLEXPORT bool IsBuiltinCatalogServerName(const char *serverName);

pg_lake_engine/src/utils/catalog_type.c

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ IsCatalogOwnedByExtension(const char *catalog)
125125
* IsRestCatalog returns true if the catalog name identifies a REST catalog.
126126
* This includes the built-in 'rest' literal and any user-created
127127
* iceberg_catalog server whose TYPE is 'rest'.
128+
*
129+
* The internal built-in server names (e.g. "pg_lake_rest_catalog") are
130+
* deliberately rejected: they are implementation details and must not be
131+
* usable as catalog= option values on CREATE TABLE. Users always type
132+
* the short name "rest", which is mapped to the long server name only
133+
* inside the resolution layer.
128134
*/
129135
bool
130136
IsRestCatalog(const char *catalog)
@@ -135,6 +141,9 @@ IsRestCatalog(const char *catalog)
135141
if (pg_strcasecmp(catalog, REST_CATALOG_NAME) == 0)
136142
return true;
137143

144+
if (IsBuiltinCatalogServerName(catalog))
145+
return false;
146+
138147
/* Try to look up a server with this name */
139148
bool missingOK = true;
140149
ForeignServer *server = GetForeignServerByName(catalog, missingOK);
@@ -147,6 +156,68 @@ IsRestCatalog(const char *catalog)
147156
if (strcmp(fdw->fdwname, ICEBERG_CATALOG_FDW_NAME) != 0)
148157
return false;
149158

159+
/*
160+
* Any iceberg_catalog server reaching this point is user-created, and
161+
* ValidateIcebergCatalogServerDDL forces all user-created iceberg_catalog
162+
* servers to TYPE 'rest'.
163+
*/
150164
Assert(pg_strcasecmp(server->servertype, REST_CATALOG_NAME) == 0);
151165
return true;
152166
}
167+
168+
169+
/*
170+
* ResolveCatalogServerName maps a user-facing catalog identifier to the
171+
* actual pg_foreign_server.srvname.
172+
*
173+
* For the three reserved short names ('postgres', 'object_store', 'rest')
174+
* the result is the corresponding pre-created built-in server name.
175+
* Any other input is returned unchanged (user-created server names match
176+
* their catalog= option value verbatim).
177+
*
178+
* The returned pointer is either a string literal or the input pointer;
179+
* callers must not free it.
180+
*/
181+
const char *
182+
ResolveCatalogServerName(const char *catalog)
183+
{
184+
if (catalog == NULL)
185+
return NULL;
186+
187+
if (pg_strcasecmp(catalog, REST_CATALOG_NAME) == 0)
188+
return PG_LAKE_REST_CATALOG_SERVER_NAME;
189+
if (pg_strcasecmp(catalog, POSTGRES_CATALOG_NAME) == 0)
190+
return PG_LAKE_POSTGRES_CATALOG_SERVER_NAME;
191+
if (pg_strcasecmp(catalog, OBJECT_STORE_CATALOG_NAME) == 0)
192+
return PG_LAKE_OBJECT_STORE_CATALOG_SERVER_NAME;
193+
194+
return catalog;
195+
}
196+
197+
198+
/*
199+
* IsBuiltinCatalogServerName returns true if the given name matches one
200+
* of the three pre-created built-in iceberg_catalog servers.
201+
*
202+
* Comparison is case-insensitive: both PostgreSQL-parsed identifiers
203+
* (already downcased by the parser unless quoted) and free-form string
204+
* literals supplied as catalog= option values flow through this helper,
205+
* and we want to reject typos like 'PG_LAKE_REST_CATALOG' just as
206+
* firmly as the canonical form.
207+
*
208+
* This is the long-name counterpart to IsCatalogOwnedByExtension, which
209+
* operates on the user-facing short names. Used by the DDL protection
210+
* hook to lock down ALTER/RENAME/OWNER on the extension's structural
211+
* anchors and by create_table.c to reject the long names as catalog=
212+
* option values.
213+
*/
214+
bool
215+
IsBuiltinCatalogServerName(const char *serverName)
216+
{
217+
if (serverName == NULL)
218+
return false;
219+
220+
return pg_strcasecmp(serverName, PG_LAKE_REST_CATALOG_SERVER_NAME) == 0 ||
221+
pg_strcasecmp(serverName, PG_LAKE_POSTGRES_CATALOG_SERVER_NAME) == 0 ||
222+
pg_strcasecmp(serverName, PG_LAKE_OBJECT_STORE_CATALOG_SERVER_NAME) == 0;
223+
}

pg_lake_iceberg/pg_lake_iceberg--3.3--3.4.sql

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,58 @@ CREATE FOREIGN DATA WRAPPER iceberg_catalog
2525
VALIDATOR lake_iceberg.iceberg_catalog_validator;
2626

2727
GRANT USAGE ON FOREIGN DATA WRAPPER iceberg_catalog TO lake_write;
28+
29+
/*
30+
* Built-in catalog servers.
31+
*
32+
* These three servers are pre-created as structural anchors for the
33+
* pg_depend dependency edges that iceberg tables record against their
34+
* catalog server. They are extension-owned and immutable: ALTER, DROP,
35+
* RENAME, and OWNER changes on them are all blocked. Configuration
36+
* for the built-in catalogs lives in GUCs, not in server options.
37+
*
38+
* Users keep typing the short names ('postgres', 'object_store', 'rest')
39+
* as the catalog= option value on CREATE TABLE; ResolveCatalogServerName
40+
* maps short -> long at server lookup time. The long names are prefixed
41+
* so they cannot collide with names users may already have in their
42+
* databases (e.g. a postgres_fdw server literally named 'postgres').
43+
*
44+
* Pre-flight: error early with a clear hint if any of the long names is
45+
* already in use. This prevents a confusing "server already exists"
46+
* mid-upgrade.
47+
*/
48+
DO $do$
49+
DECLARE
50+
conflicting text;
51+
BEGIN
52+
SELECT srvname INTO conflicting
53+
FROM pg_foreign_server
54+
WHERE srvname IN ('pg_lake_postgres_catalog',
55+
'pg_lake_object_store_catalog',
56+
'pg_lake_rest_catalog')
57+
LIMIT 1;
58+
59+
IF conflicting IS NOT NULL THEN
60+
RAISE EXCEPTION
61+
'pg_lake_iceberg upgrade conflicts with existing foreign server %', conflicting
62+
USING HINT = 'Drop or rename the server and re-run ALTER EXTENSION pg_lake_iceberg UPDATE. '
63+
'pg_lake_iceberg reserves the names pg_lake_postgres_catalog, '
64+
'pg_lake_object_store_catalog, and pg_lake_rest_catalog for internal use.';
65+
END IF;
66+
END $do$;
67+
68+
CREATE SERVER pg_lake_postgres_catalog
69+
TYPE 'postgres'
70+
FOREIGN DATA WRAPPER iceberg_catalog;
71+
72+
CREATE SERVER pg_lake_object_store_catalog
73+
TYPE 'object_store'
74+
FOREIGN DATA WRAPPER iceberg_catalog;
75+
76+
CREATE SERVER pg_lake_rest_catalog
77+
TYPE 'rest'
78+
FOREIGN DATA WRAPPER iceberg_catalog;
79+
80+
GRANT USAGE ON FOREIGN SERVER pg_lake_postgres_catalog TO lake_write;
81+
GRANT USAGE ON FOREIGN SERVER pg_lake_object_store_catalog TO lake_write;
82+
GRANT USAGE ON FOREIGN SERVER pg_lake_rest_catalog TO lake_write;

0 commit comments

Comments
 (0)