Add Iceberg lake-table DDL: CREATE ICEBERG TABLE, FOREIGN CATALOG, FOREIGN VOLUME (kernel scaffolding)#1842
Conversation
Introduce the three system catalogs backing Iceberg lake-table DDL: - pg_foreign_catalog: named foreign catalog bound to a foreign server - pg_foreign_volume: named foreign volume bound to a foreign server - pg_lake_table: per-relation lake-table metadata (type, catalog, volume, options) Register the headers in the catalog Makefile and bump CATALOG_VERSION_NO. No code references the catalogs yet; DDL/commands land in later commits. OIDs 8549-8558 / 9901-9902 verified free via unused_oids; duplicate_oids clean.
Add three statement parse nodes and their hand-maintained node-support plumbing (copy/equal/out/read + fast serialization), mirroring the existing CreateDirectoryTableStmt and CreateForeignServerStmt patterns: - CreateLakeTableStmt (CREATE ICEBERG TABLE ...; embeds CreateStmt) - CreateForeignCatalogStmt (CREATE FOREIGN CATALOG ...) - CreateForeignVolumeStmt (CREATE FOREIGN VOLUME ...) Nodes are not yet produced by the grammar or dispatched; grammar and command handling land in later commits. ObjectType additions are deferred to the command commit to keep exhaustive switches complete.
Add the ICEBERG and VOLUME unreserved keywords and the CREATE-side
grammar productions that build the parse nodes from the previous commit:
- CREATE ICEBERG TABLE name (cols) [FOREIGN CATALOG c] [FOREIGN VOLUME v]
OPTIONS (...) -> CreateLakeTableStmt (forced DISTRIBUTED RANDOMLY)
- CREATE FOREIGN CATALOG name SERVER s OPTIONS (...) -> CreateForeignCatalogStmt
- CREATE FOREIGN VOLUME name SERVER s OPTIONS (...) -> CreateForeignVolumeStmt
Statements parse but are not yet dispatched; command handling, ObjectType
entries and DROP support land in the next commit. Verified: bison reports
no grammar conflicts; the statements parse and reach ProcessUtility.
Make the foreign catalog and foreign volume DDL commands functional on top of the previously added grammar, parse nodes and system catalogs: * CreateForeignCatalog()/CreateForeignVolume() insert into pg_foreign_catalog/pg_foreign_volume, check server existence and USAGE privilege, record dependencies on the server and owner, and dispatch to segments with preassigned OIDs. * Lookup helpers get_foreign_catalog_oid(), get_foreign_volume_oid() and GetForeignVolumeByName(); both objects are unique on (name, server). * New object infrastructure: OBJECT_FOREIGN_CATALOG/OBJECT_FOREIGN_VOLUME and OCLASS_FOREIGN_CATALOG/OCLASS_FOREIGN_VOLUME with handlers in all exhaustive switches (objectaddress, dependency, aclchk, event trigger, seclabel, dropcmds, alter). Ownership checks go through the generic object_ownercheck() via the new ObjectProperty entries. * Four new syscaches: FOREIGNCATALOGNAME/FOREIGNCATALOGOID and FOREIGNVOLUMENAMESERVER/FOREIGNVOLUMEOID. * DROP CATALOG / DROP VOLUME grammar via drop_type_name, going through the regular RemoveObjects() path with dependency handling, so DROP SERVER ... CASCADE also removes dependent catalogs and volumes. * utility.c dispatch and CREATE/DROP FOREIGN CATALOG|VOLUME command tags.
Make CREATE ICEBERG TABLE functional on top of the existing grammar, parse nodes and pg_lake_table catalog: * laketablecmds.c: CreateLakeTable() inserts the pg_lake_table entry after DefineRelation and records dependencies on the table's foreign catalog and volume; ValidateLakeTableOptions() runs the same resolution on the QD before DefineRelation so validation failures don't surface as QE-annotated errors; RemoveLakeTableEntry() cleans up on drop (hooked into heap_drop_with_catalog). * iceberg_default_catalog / iceberg_default_volume GUCs (synchronized to QEs) provide defaults when CREATE ICEBERG TABLE has no CATALOG or VOLUME clause; their check hooks verify the object exists. * The iceberg table access method is resolved strictly by name (get_table_am_oid) and is expected to be provided by a datalake extension; without it, CREATE ICEBERG TABLE fails up front with a hint. The kernel does not hardcode any extension name or AM OID. * Guard rails: reject the iceberg AM for every creation path other than CreateLakeTableStmt (CREATE TABLE ... USING iceberg, CTAS, matview, default_table_access_method, partition children), reject ALTER TABLE ... SET ACCESS METHOD to or from iceberg, and reject SET DISTRIBUTED BY on lake tables, which must stay DISTRIBUTED RANDOMLY. A relation created with the iceberg AM but without its pg_lake_table metadata would be unusable and undroppable. * utility.c dispatch (transformCreateStmt works on the embedded CreateStmt) and the CREATE LAKE TABLE command tag.
pg_dump cannot reproduce lake tables (their data lives in external object storage managed through the iceberg access method's foreign catalog and volume), so skip them with a warning, matching how other unsupported access methods are handled. The access method is matched by name, not by a hardcoded OID. psql tab completion learns CREATE FOREIGN CATALOG/VOLUME ... SERVER ... OPTIONS, CREATE ICEBERG TABLE, DROP CATALOG/VOLUME with CASCADE/ RESTRICT, and completes foreign catalog and volume names after the CATALOG and VOLUME keywords.
Add a lake_table test to the greenplum_schedule covering the new DDL end to end: CREATE/DROP FOREIGN CATALOG and FOREIGN VOLUME (duplicates, IF NOT EXISTS/IF EXISTS, missing server), segment dispatch, object descriptions, CREATE ICEBERG TABLE with explicit CATALOG/VOLUME clauses and via the iceberg_default_catalog/volume GUCs, the forced random distribution policy, every iceberg-AM misuse guard, ownership checks, and dependency behavior (RESTRICT errors, CASCADE, pg_lake_table cleanup on drop). The iceberg access method is simulated with a heap-backed CREATE ACCESS METHOD, since the kernel resolves it by name and the real AM comes from a datalake extension. Refresh expected output of tests that enumerate system catalogs for the three new ones: misc_sanity (pg_lake_table's toast-less varlena columns), sanity_check (catalog list), and oidjoins (BKI_LOOKUP references, including pg_lake_table.ltforeign_catalog pointing at pg_foreign_catalog).
Address review: new community-authored files should carry the standard Apache-2.0 header instead of the PostgreSQL boilerplate.
Match the community convention (see contrib/pax_storage pax_gbench.cc): the Apache-2.0 license block comes first, followed by the file name and IDENTIFICATION.
| CREATE SERVER lake_test_srv2 FOREIGN DATA WRAPPER lake_test_fdw; | ||
|
|
||
| -- CREATE FOREIGN CATALOG | ||
| CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv OPTIONS (type 'hive', uri 'thrift://localhost:9083'); |
There was a problem hiding this comment.
What about making type as a required parameter to make more strict checks? For example
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'hive' URI 'thrift://localhost:9083';
When I pass options I can pass nonexistent type and invalid option name, and the query is executed without errors
postgres=# CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv OPTIONS (type 'nonexistent_type', uriii 'thrift://localhost:9083');
CREATE FOREIGN CATALOG
postgres=# table pg_foreign_catalog;
oid | fcname | fcowner | fcserver | fcoptions
-------+---------------+---------+----------+-------------------------------------------------------
17022 | lake_test_cat | 10 | 17019 | {type=nonexistent_type,uriii=thrift://localhost:9083}
(1 row)
postgres=#
There was a problem hiding this comment.
I fully agree. However, I haven’t implemented validation at this stage because the syntax is still incomplete and subject to frequent changes. Given this, would you recommend we add validation now, or hold off until the entire workflow is fully functional before enforcing validation rules?
There was a problem hiding this comment.
I recommend to make syntax without the OPTIONS keyword, if possible, because in this case validation will be implemented in grammar. So the validation will be an integral part of the syntax implementation.
There was a problem hiding this comment.
That might be difficult. We need to support various external catalogs with numerous distinct parameter configurations. Therefore, using SQL options would be the optimal approach.
There was a problem hiding this comment.
May be. But I think every catalog has its type. So type is not an option, but a required property, which should be in pg_foreign_catalog as a column and in grammar
There was a problem hiding this comment.
The currently supported types are hive, hdfs, polaris, s3 and builtin.
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'builtin';
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 's3';
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'hive';
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'polaris';
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'hdfs';
Is it correct like this?
There was a problem hiding this comment.
What about type name without quotes around?
CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE builtin;
There was a problem hiding this comment.
I don't recommend this approach, as it would result in too many dedicated catalog creation syntaxes. We may also support JDBC catalog and other user-defined catalog types down the line. Creating a separate syntax for each type would clearly be redundant.
| DROP CATALOG lake_test_cat; -- fail, already gone | ||
| DROP CATALOG IF EXISTS lake_test_cat; -- skip with notice | ||
| DROP VOLUME lake_test_vol; | ||
| DROP VOLUME lake_test_vol; -- fail, already gone |
There was a problem hiding this comment.
When I have created foreign table, I cannot drop it using drop table, I should use DROP FOREIGN TABLE. But in the case of foreign catalog I should drop it using drop catalog without the foreign keyword. So I suggest to replace DROP CATALOG with DROP FOREIGN CATALOG and DROP VOLUME with DROP FOREIGN VOLUME for unification with existing syntax
postgres=# create foreign table ttt(i int, j int) server lake_test_srv;
CREATE FOREIGN TABLE
postgres=# drop table ttt;
ERROR: "ttt" is not a table
HINT: Use DROP FOREIGN TABLE to remove a foreign table.
postgres=# DROP FOREIGN CATALOG lake_test_cat;
ERROR: syntax error at or near "CATALOG"
LINE 1: DROP FOREIGN CATALOG lake_test_cat;
^
postgres=#
There was a problem hiding this comment.
I suggest to add DROP ICEBERG TABLE as a pair for CREATE ICEBERG TABLE too.
postgres=# CREATE ICEBERG TABLE lake_test_t0 (a int) CATALOG lake_test_cat VOLUME lake_test_vol;
CREATE LAKE TABLE
postgres=# DROP ICEBERG TABLE lake_test_t0;
ERROR: syntax error at or near "ICEBERG"
LINE 1: DROP ICEBERG TABLE lake_test_t0;
^
postgres=#
Address review: invert the if_not_exists check and error out early so the skip path is no longer nested in an else branch, in both CreateForeignCatalog and CreateForeignVolume.
| GetIcebergTableAmOid(bool missing_ok) | ||
| { | ||
| return get_table_am_oid(ICEBERG_TABLE_AM_NAME, missing_ok); | ||
| } |
There was a problem hiding this comment.
In the b4704be commit description I read " The iceberg table access method is resolved strictly by name (get_table_am_oid) and is expected to be provided by a datalake extension; without it, CREATE ICEBERG TABLE fails up front with a hint. The kernel does not hardcode any extension name or AM OID."
In my opinion, it is strange to have queries (like CREATE ICEBERG TABLE) in the core which are useless without extensions. What about default iceberg AM which can be replaced (using USING) with AM from some extension? Without default AM we cannot test the code properly.
There was a problem hiding this comment.
- Does this AM refer to the minimal Iceberg AM implementation that does not support scans or inserts? Is that correct?
- One more question: CREATE ICEBERG TABLE USING
Is this syntax designed to support extensions via other frameworks? From my understanding, there should be no need to specify USING am if we're already using CREATE ICEBERG TABLE, right?
| * extension's OID assignments. | ||
| */ | ||
| bool | ||
| RelationIsIcebergTable(Relation rel) |
There was a problem hiding this comment.
What about adding RELKIND_ICEBERG constant like RELKIND_FOREIGN_TABLE and checking rel->rd_rel->relkind here to make it possible to use more that one extension with AM?
There was a problem hiding this comment.
Adding RELKIND_ICEBERG requires modifications across all reference points, which will be a substantial change. Do we plan to support multiple AMs in the future? From my understanding, CREATE ICEBERG statements natively only create Iceberg-related tables. you mean like hudi, Paimon ?,Could you share your thoughts on this?
There was a problem hiding this comment.
If that’s the case, a syntax like CREATE LAKEHOUSE xxx USING xxx would be more appropriate.
There was a problem hiding this comment.
If we can add AM using an extension, then we can have more than one such extensions. User may want to install several extensions. I'm not sure if several iceberg extensions installation is really useful. But your idea about possible support of other formats (CREATE LAKEHOUSE xxx USING xxx) seems to be very interesting
|
|
||
| /* If this is a lake table, remove its pg_lake_table entry */ | ||
| if (RelationIsIcebergTable(rel)) | ||
| RemoveLakeTableEntry(relid); |
There was a problem hiding this comment.
can RemoveLakeTableEntry put into extension via object_access_hook ?
Summary
Kernel-side DDL scaffolding for Iceberg lake tables, per the design proposal in #1683 (this PR is the "core syntax" milestone of the roadmap posted there). It adds the system catalogs, parse nodes, grammar, commands, and client-tool awareness that a datalake provider extension builds on. No table AM implementation or provider is included — those are later milestones (agent / FDW phases in the roadmap).
What's included (structured for commit-by-commit review)
pg_foreign_catalog,pg_foreign_volume(metadata service / storage location handles, hanging offpg_foreign_server),pg_lake_table(per-relation lake metadata keyed by relid). All three have TOAST tables; name indexes are single-column unique (see "Design decisions").CreateLakeTableStmt(embedsCreateStmt),CreateForeignCatalogStmt,CreateForeignVolumeStmt+ hand-maintained node-support functions.CREATE ICEBERG TABLE ... CATALOG ... VOLUME ... OPTIONS (...),CREATE FOREIGN CATALOG|VOLUME ... SERVER ...,DROP CATALOG|VOLUMEviadrop_type_name.iceberg_default_catalog/iceberg_default_volumeGUCs), validates the statement uses theicebergAM, creates the relation + TOAST +pg_lake_tablerow + dependencies; guardsALTER TABLE SET ACCESS METHOD/SET DISTRIBUTED BYboth directions; lake tables are forcedDISTRIBUTED RANDOMLY.pg_dumpskips iceberg tables (by AM name, with a warning — same pattern as PAX); psql tab completion.lake_tableregression ingreenplum_schedule(DDL lifecycle, duplicate/USING rejection, TOAST wide rows, ownership, dependency/CASCADE, QD/QE dispatch checks) + catalog-expected refresh acrossregress,singlenode_regress, and pax copies.Design decisions to review
get_table_am_oid("iceberg")); no hardcoded AM OID and no extension-name checks. Everything degrades gracefully (with a hint) when no provider is installed.CreateLakeTable()runs afterDefineRelation()and ends withCommandCounterIncrement()+InvokeObjectPostCreateHook(LakeTableRelationId, ...)— a provider performs remote Iceberg creation from that object-access hook (on QD and QEs), where catalog/volume/options metadata is already visible.relation_set_new_filelocatorremains local-storage-only.pg_foreign_server— every reference syntax (DROP CATALOG x, theCATALOG xclause, the GUCs) identifies them by bare name, so the uniqueness scope matches what the syntax can express. (The alternative — per-server names like user mappings — would require server-qualified reference syntax everywhere.)CREATE ICEBERG TABLErejects aUSINGclause naming any other AM; without it the statement impliesUSING iceberg.Known limitations / open questions (deliberately out of scope here)
USAGEon its server. Referencing one fromCREATE ICEBERG TABLEcurrently checks existence only — whether that should requireUSAGEon the underlying server, or dedicated ACLs (GRANT USAGE ON CATALOG), is an open question we'd like reviewer input on.default_tablespacew.r.t. objects dropped afterSET.Discussion: #1683