andr-sokolov commented on code in PR #1842:
URL: https://github.com/apache/cloudberry/pull/1842#discussion_r3585478935
##########
src/include/tcop/cmdtaglist.h:
##########
@@ -176,10 +179,13 @@ PG_CMDTAG(CMDTAG_DROP_DOMAIN, "DROP DOMAIN", true, false,
false)
PG_CMDTAG(CMDTAG_DROP_DYNAMIC_TABLE, "DROP DYNAMIC TABLE", true, false, false)
PG_CMDTAG(CMDTAG_DROP_EVENT_TRIGGER, "DROP EVENT TRIGGER", false, false, false)
PG_CMDTAG(CMDTAG_DROP_EXTENSION, "DROP EXTENSION", true, false, false)
+PG_CMDTAG(CMDTAG_DROP_FOREIGN_CATALOG, "DROP FOREIGN CATALOG", true, false,
false)
PG_CMDTAG(CMDTAG_DROP_FOREIGN_DATA_WRAPPER, "DROP FOREIGN DATA WRAPPER", true,
false, false)
PG_CMDTAG(CMDTAG_DROP_FOREIGN_TABLE, "DROP FOREIGN TABLE", true, false, false)
+PG_CMDTAG(CMDTAG_DROP_FOREIGN_VOLUME, "DROP FOREIGN VOLUME", true, false,
false)
PG_CMDTAG(CMDTAG_DROP_FUNCTION, "DROP FUNCTION", true, false, false)
PG_CMDTAG(CMDTAG_DROP_INDEX, "DROP INDEX", true, false, false)
+PG_CMDTAG(CMDTAG_DROP_LAKE_TABLE, "DROP LAKE TABLE", true, false, false)
Review Comment:
When I run a `CREATE ICEBERG TABLE` query, I get output `CREATE LAKE TABLE`.
The same replacement `ICEBERG` with `LAKE` happens in the case of `DROP ICEBERG
TABLE`.
```
postgres=# CREATE ICEBERG TABLE lake_test_t1 (a int, b text) CATALOG
lake_test_cat VOLUME lake_test_vol OPTIONS (fileformat 'parquet');
CREATE LAKE TABLE
postgres=# DROP ICEBERG TABLE IF EXISTS lake_test_t1;
DROP LAKE TABLE
postgres=#
```
Usually the output is the beginning of CREATE or DROP query. May be it means
that we should replace `CREATE ICEBERG TABLE` with `CREATE LAKE TABLE ... TYPE
'ICEBERG'` or `CREATE LAKE TABLE ... USING ICEBERG`? We have the
laketablecmds.c and laketablecmds.h files, the `T_CreateLakeTableStmt` node
with the `table_type` field and the `pg_lake_table` catalog table with
`lttable_type` column. I think it will be cool if the syntax will correspond to
data structures and names in the source code.
##########
doc/src/sgml/ref/create_iceberg_table.sgml:
##########
@@ -0,0 +1,193 @@
+<!--
+doc/src/sgml/ref/create_iceberg_table.sgml
+PostgreSQL documentation
+-->
+
+<refentry id="sql-createicebergtable">
+ <indexterm zone="sql-createicebergtable">
+ <primary>CREATE ICEBERG TABLE</primary>
+ </indexterm>
+
+ <refmeta>
+ <refentrytitle>CREATE ICEBERG TABLE</refentrytitle>
+ <manvolnum>7</manvolnum>
+ <refmiscinfo>SQL - Language Statements</refmiscinfo>
+ </refmeta>
+
+ <refnamediv>
+ <refname>CREATE ICEBERG TABLE</refname>
+ <refpurpose>define a new Iceberg table</refpurpose>
+ </refnamediv>
+
+ <refsynopsisdiv>
+<synopsis>
+CREATE ICEBERG TABLE [ IF NOT EXISTS ] <replaceable
class="parameter">table_name</replaceable> ( [
+ <replaceable class="parameter">column_name</replaceable> <replaceable
class="parameter">data_type</replaceable> [, ... ]
+ ] )
+ [ CATALOG <replaceable class="parameter">catalog_name</replaceable> ]
+ [ VOLUME <replaceable class="parameter">volume_name</replaceable> ]
+ [ OPTIONS ( <replaceable class="parameter">option</replaceable>
'<replaceable class="parameter">value</replaceable>' [, ...] ) ]
+ [ DISTRIBUTED BY ( <replaceable class="parameter">column</replaceable> [,
... ] ) | DISTRIBUTED RANDOMLY | DISTRIBUTED REPLICATED ]
Review Comment:
It is written in the test comment: "Lake tables are always DISTRIBUTED
RANDOMLY". Can we remove this line from syntax? This line seems meaningless.
##########
src/test/regress/sql/lake_table.sql:
##########
@@ -0,0 +1,155 @@
+--
+-- Test lake table DDL: FOREIGN CATALOG, FOREIGN VOLUME, ICEBERG 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%' ORDER BY 1;
Review Comment:
Why do we need `ORDER BY 1` here and below? Tuples order doesn't matter for
regression tests
--
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]
