Skip to content

Support TYPE 'object_store' servers; make read_only a server option#320

Draft
sfc-gh-npuka wants to merge 27 commits into
naisila/catalog_user_mappingfrom
naisila/object_store
Draft

Support TYPE 'object_store' servers; make read_only a server option#320
sfc-gh-npuka wants to merge 27 commits into
naisila/catalog_user_mappingfrom
naisila/object_store

Conversation

@sfc-gh-npuka
Copy link
Copy Markdown
Collaborator

@sfc-gh-npuka sfc-gh-npuka commented Apr 17, 2026 •
edited
Loading

NOTE: not ready for review yet

Allow CREATE SERVER ... TYPE 'object_store' FOREIGN DATA WRAPPER iceberg_catalog, mirroring the existing TYPE 'rest' pattern. Each server can carry its own location_prefix and read_only options, enabling shared writer/reader topologies where multiple Postgres instances point at the same S3 path.

Key changes:

  • ValidateIcebergCatalogServerDDL now accepts TYPE 'object_store' (TYPE 'postgres' remains rejected).
  • Add IsObjectStoreCatalog() in catalog_type.c, paralleling IsRestCatalog(); update HasObjectStoreCatalogTableOption to use it so that tables with catalog='<named_server>' resolve correctly.
  • Add ObjectStoreCatalogOptions struct and resolution function that reads server options with GUC fallback.
  • Add read_only as a server-level option (valid for both REST and object_store servers). Server read_only propagates to all tables unless overridden; table read_only='false' on a read_only server is an error.
  • Update create_table.c to resolve location_prefix from named object_store servers instead of requiring the GUC.
  • Update IcebergDefaultCatalogCheckHook to accept named object_store servers.
  • Update catalog export SPI query to include tables on named object_store servers.

Checklist

  • 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-npuka
Add iceberg_catalog FDW and refactor REST catalog for multi-server su…
32731a1 
…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>
@sfc-gh-npuka
Recognize server-based REST catalogs in catalog type detection
6cc0b78 
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>
@sfc-gh-npuka
Support server-based catalogs in option validation and table creation
10b66ec 
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>
@sfc-gh-npuka
Track per-table REST catalog connections in transaction handling
bf3f5e8 
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>
@sfc-gh-npuka
Add tests for iceberg_catalog server configuration
dc60bc3 
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>
@sfc-gh-npuka
Add ProtectExtensionCatalogServersHandler
f39d59f 
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>
@sfc-gh-npuka
Address review
1e45830 
- 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>
@sfc-gh-npuka
Allow extension owned rest catalog to alter server options
93fc158 
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>
@sfc-gh-npuka
Address review part 1
260dc78 
- 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>
@sfc-gh-npuka
Address review part 2
d3a6250 
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>
@sfc-gh-npuka
Block reserved catalog names and fix prefix-match comparisons
e94d080 
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>
@sfc-gh-npuka
Address Onder's review
e12e69a 
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>
@sfc-gh-npuka
Treat postgres, object_store, and rest as built-in names, not servers
8fd726f 
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>
@sfc-gh-npuka
Use catalog_name server option for REST catalog tables
b2024c2 
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>
@sfc-gh-npuka
Unrelated to PR - minor fix for reordering of tests
4566882 
Last 3 commits added/removed enough tests in other files
to shift the pytest-split group boundaries, moving
test_create_extension_table_access_method away from
test_create_drop_query_engine (which previously cleaned up
dependent extensions). The test was always fragile as it
assumed no dependent extensions were installed.

Adding CASCADE makes it work regardless of ordering of tests.

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Cleanup from rebase and fix style
f1bb78f 
Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Address Aykut's latest review
d153edc 
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>
@sfc-gh-npuka
Move credentials from server options to user mapping in iceberg_catal…
31f11c8 
…og validator

Move secrets (client_id, client_secret) to CREATE USER MAPPING OPTIONS.

- Split iceberg_catalog_server_options and iceberg_catalog_user_mapping_options
- Add UserMappingRelationId handling to iceberg_catalog_validator

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Add multi-tier credential resolution: user mapping, catalogs.conf, GU…
7f4a5e4 
…C fallback

Implement the credential resolution chain for server-based REST catalogs:
  1. CREATE USER MAPPING (per-user, cached by relcache)
  2. $PGDATA/catalogs.conf (platform-provided, re-read each time)
  3. GUC variables (backward compatibility)

New functions:
- LookupUserMappingOptions: queries pg_user_mapping syscache without
  erroring when no mapping exists (unlike GetUserMapping)
- ReadCatalogsConfCredentials: parses $PGDATA/catalogs.conf using
  ParseConfigFp, matching dotted keys like horizon.client_id

Rewrite GetRestCatalogOptionsFromCatalog with two phases:
  Phase 1: non-secret config from server options
  Phase 2: credentials from user mapping > catalogs.conf > GUCs

Update extension upgrade SQL with revised documentation showing
user-defined (CREATE USER MAPPING) and platform-provided (catalogs.conf)
catalog examples.

- scope is accepted in both server and user mapping (user mapping wins)

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Update tests for user mapping credentials and catalogs.conf resolution
799244a 
- Move client_id/client_secret from server OPTIONS to CREATE USER MAPPING
  in all tests that previously used server-level credentials
- Add test_reject_secrets_in_server_options: verifies client_id in server
  OPTIONS is now rejected
- Add user mapping tests: valid options, PUBLIC mapping, rejection of
  unknown and server-level options in user mapping context
- Add catalogs.conf tests with pg_data_dir and catalogs_conf fixtures:
  credentials from config file, user mapping override, missing credentials
  error, and scope from config file
- Update ALTER SERVER test to use scope instead of client_id

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Scrub user mapping secrets in-place via ProcessUtility handler
db9e9c5 
When CREATE USER MAPPING or ALTER USER MAPPING targets an
iceberg_catalog server, overwrite client_id and client_secret values
with asterisks directly in the queryString buffer.  This prevents
credential leakage in pg_stat_statements and similar.

Implementation:

- iceberg_catalog_secret_options: separates secret names (client_id,
  client_secret) from the full user mapping option list so scope
  remains visible.
- ScrubUserMappingSecrets: walks DefElem options, uses DefElem.location
  to find each secret quoted value in the query string, and overwrites
  value characters with * while preserving quotes and string length.
- ScrubIcebergUserMappingHandler: detects user mapping DDL targeting
  iceberg_catalog servers, calls ScrubUserMappingSecrets, returns false
  so normal processing continues.

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Address review
7f71fc2 
Rename ScrubIcebergUserMappingHandler to RedactRestCatalogUserMappingSecrets
and move its registration from pg_lake_iceberg to pg_lake_table, alongside
BlockDDLOnExtensionCatalogs.

Reorder credential resolution in GetRestCatalogConnectionFromServer so the
code reads in ascending priority: GUCs (defaults) → catalogs.conf → user
mapping. Previously catalogs.conf was checked after user mapping with
NULL guards to avoid overriding it.

Guard catalogs.conf parser against malformed entries like "rest." (dot
with no key after it) by checking item->name[serverNameLen + 1] != '\0'.

Add test for catalogs.conf on the extension-owned rest server.

Add DDL-level test for per-user scope isolation: two roles with different
scopes on the same server, verified via pg_user_mapping.

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Add configurable catalogs.conf path via GUC
7f645e2 
New GUC pg_lake_iceberg.catalogs_conf_path controls the location of the
catalog credentials file. Defaults to "catalogs.conf", which resolves to
$PGDATA/catalogs.conf. Absolute paths are used as-is. The GUC is
PGC_SIGHUP (reloadable) and superuser-only.

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Use currentChar in ScrubUserMappingSecrets for readability
747b2e4 
Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Rename ScrubUserMappingSecrets to RedactUserMappingSecrets; fix style
507450b 
Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka
Make read_only a server option; Support TYPE 'object_store' servers
fb0da6f 
Allow CREATE SERVER ... TYPE 'object_store' FOREIGN DATA WRAPPER
iceberg_catalog, mirroring the existing TYPE 'rest' pattern. Each
server can carry its own location_prefix and read_only options,
enabling shared writer/reader topologies where multiple Postgres
instances point at the same S3 path.

Key changes:
- ValidateIcebergCatalogServerDDL now accepts TYPE 'object_store'
  (TYPE 'postgres' remains rejected).
- Add IsObjectStoreCatalog() in catalog_type.c, paralleling
  IsRestCatalog(); update HasObjectStoreCatalogTableOption to use it
  so that tables with catalog='<named_server>' resolve correctly.
- Add ObjectStoreCatalogOptions struct and resolution function that
  reads server options with GUC fallback.
- Add read_only as a server-level option (valid for both REST and
  object_store servers). Server read_only propagates to all tables
  unless overridden; table read_only='false' on a read_only server
  is an error.
- Update create_table.c to resolve location_prefix from named
  object_store servers instead of requiring the GUC.
- Update IcebergDefaultCatalogCheckHook to accept named object_store
  servers.
- Update catalog export SPI query to include tables on named
  object_store servers.

Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka sfc-gh-npuka force-pushed the naisila/object_store branch from 5d0b9ef to fb0da6f Compare April 17, 2026 13:52
@sfc-gh-npuka
Fix style
706d708 
Signed-off-by: sfc-gh-npuka <naisila.puka@snowflake.com>
@sfc-gh-npuka sfc-gh-npuka force-pushed the naisila/catalog_user_mapping branch from 507450b to 8ed0b8e Compare May 26, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sfc-gh-npuka