MisterRaindrop commented on code in PR #1842:
URL: https://github.com/apache/cloudberry/pull/1842#discussion_r3662263724
##########
src/test/regress/sql/lake_table.sql:
##########
@@ -0,0 +1,158 @@
+--
+-- Test lake table DDL: FOREIGN CATALOG, FOREIGN VOLUME, LAKE TABLE
+--
+
+-- Display the lake table catalogs
+\d+ pg_foreign_catalog
+\d+ pg_foreign_volume
+\d+ pg_lake_table
+
+-- Setup: foreign servers for the catalogs and volumes to hang off
+CREATE FOREIGN DATA WRAPPER lake_test_fdw;
+CREATE SERVER lake_test_srv FOREIGN DATA WRAPPER lake_test_fdw;
+CREATE SERVER lake_test_srv2 FOREIGN DATA WRAPPER lake_test_fdw;
+
+-- CREATE FOREIGN CATALOG: TYPE is a required first-class property
+CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'hive' OPTIONS
(uri 'thrift://localhost:9083');
+CREATE FOREIGN CATALOG lake_test_notype SERVER lake_test_srv; --
fail, TYPE is required
+CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv TYPE 'hive';
-- fail, duplicate
+CREATE FOREIGN CATALOG IF NOT EXISTS lake_test_cat SERVER lake_test_srv TYPE
'hive'; -- skip with notice
+-- catalog names are global: the same name on another server is still a
duplicate
+CREATE FOREIGN CATALOG lake_test_cat SERVER lake_test_srv2 TYPE 'hive';
-- fail, duplicate
+CREATE FOREIGN CATALOG IF NOT EXISTS lake_test_cat SERVER lake_test_srv2 TYPE
'hive'; -- skip with notice
+CREATE FOREIGN CATALOG lake_test_bad SERVER no_such_server TYPE 'hive';
-- fail, no server
+SELECT fcname, fctype, fcoptions FROM pg_foreign_catalog WHERE fcname LIKE
'lake\_test%';
+
+-- CREATE FOREIGN VOLUME
+CREATE FOREIGN VOLUME lake_test_vol SERVER lake_test_srv OPTIONS (base_path
's3://bucket/prefix');
+CREATE FOREIGN VOLUME lake_test_vol SERVER lake_test_srv;
-- fail, duplicate
+CREATE FOREIGN VOLUME IF NOT EXISTS lake_test_vol SERVER lake_test_srv;
-- skip with notice
+-- volume names are global: the same name on another server is still a
duplicate
+CREATE FOREIGN VOLUME lake_test_vol SERVER lake_test_srv2;
-- fail, duplicate
+CREATE FOREIGN VOLUME IF NOT EXISTS lake_test_vol SERVER lake_test_srv2;
-- skip with notice
+CREATE FOREIGN VOLUME lake_test_bad SERVER no_such_server;
-- fail, no server
+SELECT fvname, fvoptions FROM pg_foreign_volume WHERE fvname LIKE
'lake\_test%';
+
+-- Object descriptions
+SELECT pg_catalog.pg_describe_object('pg_foreign_catalog'::regclass, oid, 0)
+ FROM pg_foreign_catalog WHERE fcname = 'lake_test_cat';
+SELECT pg_catalog.pg_describe_object('pg_foreign_volume'::regclass, oid, 0)
+ FROM pg_foreign_volume WHERE fvname = 'lake_test_vol';
+
+-- Catalog and volume rows are dispatched to all segments
+SELECT count(DISTINCT gp_segment_id) > 1 AS on_all_segments
+ FROM gp_dist_random('pg_foreign_catalog') WHERE fcname = 'lake_test_cat';
+SELECT count(DISTINCT gp_segment_id) > 1 AS on_all_segments
+ FROM gp_dist_random('pg_foreign_volume') WHERE fvname = 'lake_test_vol';
+
+-- Without a provider extension there is no iceberg table AM
+CREATE LAKE TABLE lake_test_t0 (a int) USING ICEBERG CATALOG lake_test_cat
VOLUME lake_test_vol; -- fail with hint
Review Comment:
@yjhjstz Agreed — this looks technically feasible, so I sketched what the
fully-extension shape could look like.
Since Cloudberry's table AM API already supports per-AM reloptions
validation (`TableAmRoutine->amoptions`, unlike vanilla PG16), `WITH (catalog
..., volume ...)` can be validated by the iceberg AM itself with no kernel
changes. The catalog/volume objects could reuse the existing FDW infrastructure
(`SERVER` + `USER MAPPING`), which gives ACLs, pg_dump support, QD/QE dispatch
and dependency tracking for free:
```sql
CREATE EXTENSION pg_iceberg; -- provides the table AM, two FDWs, and hooks
CREATE SERVER hive_cat FOREIGN DATA WRAPPER iceberg_catalog
OPTIONS (type 'hive', uri 'thrift://metastore:9083');
CREATE USER MAPPING FOR alice SERVER hive_cat
OPTIONS (user '...', password '...');
CREATE SERVER s3_vol FOREIGN DATA WRAPPER iceberg_volume
OPTIONS (base_path 's3://bucket/prefix');
CREATE TABLE t (a int, b text) USING iceberg
WITH (catalog 'hive_cat', volume 's3_vol');
DROP TABLE t; -- plain DROP TABLE applies
```
A few things the extension would need to own, instead of kernel guards:
- registering table metadata and `pg_depend` entries (table → server) via
`object_access_hook`, so `DROP SERVER` stays protected and CASCADE works;
- intercepting paths that don't make sense for iceberg (CTAS, partition
children, `ALTER TABLE ... SET ACCESS METHOD`) via `ProcessUtility_hook`, and
forcing random distribution;
- the library would need to be in `shared_preload_libraries` so the hooks
are always installed.
This is essentially the pg_lake architecture, and `USER MAPPING` would also
give a cleaner credentials story than embedding keys in options. If we go this
way, the kernel scaffolding in this PR (the grammar, the three catalogs, the
guards) would no longer be needed — the work would move into the datalake
extension instead. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
