Copilot commented on code in PR #9430:
URL: https://github.com/apache/gravitino/pull/9430#discussion_r2650627746


##########
scripts/postgresql/schema-1.2.0-postgresql.sql:
##########
@@ -0,0 +1,786 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file--

Review Comment:
   There is a syntax error in the SQL comment. Line 3 has double dashes "--" 
where one comment block ends and another begins without proper spacing. This 
should be "-- or more contributor license agreements.  See the NOTICE file" 
with proper comment continuation, or the comment should be reformatted to avoid 
this issue.



##########
scripts/mysql/schema-1.2.0-mysql.sql:
##########
@@ -0,0 +1,442 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file--

Review Comment:
   There is a syntax error in the SQL comment. Line 3 has double dashes "--" 
where one comment block ends and another begins without proper spacing. This 
should be "-- or more contributor license agreements.  See the NOTICE file" 
with proper comment continuation, or the comment should be reformatted to avoid 
this issue.
   ```suggestion
   -- or more contributor license agreements.  See the NOTICE file
   ```



##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableMetaBaseSQLProvider.java:
##########
@@ -230,4 +284,57 @@ public String deleteTableMetasByLegacyTimeline(
         + TABLE_NAME
         + " WHERE deleted_at > 0 AND deleted_at < #{legacyTimeline} LIMIT 
#{limit}";
   }
+
+  public String selectTableByFullQualifiedName(
+      @Param("metalakeName") String metalakeName,
+      @Param("catalogName") String catalogName,
+      @Param("schemaName") String schemaName,
+      @Param("tableName") String tableName) {
+    return """
+        SELECT
+            sm.schema_id AS schemaId,
+            cm.catalog_id AS catalogId,
+            tm.table_id AS tableId,
+            tm.table_name AS tableName,
+            tm.metalake_id AS metalakeId,
+            tm.audit_info AS auditInfo,
+            tm.current_version AS currentVersion,
+            tm.last_version AS lastVersion,
+            tm.deleted_at AS deletedAt,
+            tvi.format AS format,
+            tvi.properties AS properties,
+            tvi.partitioning AS partitions,
+            tvi.sort_orders AS sortOrders,
+            tvi.distribution AS distribution,
+            tvi.indexes AS indexes,
+            tvi.comment AS comment
+        FROM
+            %s mm
+        INNER JOIN
+            %s cm ON mm.metalake_id = cm.metalake_id
+            AND cm.catalog_name = #{catalogName}
+            AND cm.deleted_at = 0
+        LEFT JOIN
+            %s sm ON cm.catalog_id = sm.catalog_id
+            AND sm.schema_name = #{schemaName}
+            AND sm.deleted_at = 0
+        LEFT JOIN
+            %s tm ON sm.schema_id = tm.schema_id
+            AND tm.table_name = #{tableName}
+            AND tm.deleted_at = 0
+        LEFT JOIN
+            %s tvi ON tm.table_id = tvi.table_id
+            AND tm.current_version = tvi.version
+            AND tvi.deleted_at = 0
+        WHERE
+            mm.metalake_name = #{metalakeName}
+            AND mm.deleted_at = 0;

Review Comment:
   The SQL query uses LEFT JOIN for the schema join, which means it will return 
a row even when the schema doesn't exist (with NULL schemaId). This could lead 
to incorrect behavior. The join should be an INNER JOIN for sm (schema) since a 
table cannot exist without a schema. Only the table version join (tvi) should 
be LEFT JOIN.



##########
scripts/h2/schema-1.2.0-h2.sql:
##########
@@ -0,0 +1,451 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file--
+--  distributed with this work for additional information
+-- regarding copyright ownership.  The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"). You may not use this file except in compliance
+-- with the License.  You may obtain a copy of the License at
+--
+--  http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing,
+-- software distributed under the License is distributed on an
+-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+-- KIND, either express or implied.  See the License for the
+-- specific language governing permissions and limitations
+-- under the License.
+--
+
+CREATE TABLE IF NOT EXISTS `metalake_meta` (
+                                               `metalake_id` BIGINT(20) 
UNSIGNED NOT NULL COMMENT 'metalake id',
+    `metalake_name` VARCHAR(128) NOT NULL COMMENT 'metalake name',
+    `metalake_comment` VARCHAR(256) DEFAULT '' COMMENT 'metalake comment',
+    `properties` CLOB DEFAULT NULL COMMENT 'metalake properties',
+    `audit_info` CLOB NOT NULL COMMENT 'metalake audit info',
+    `schema_version` CLOB NOT NULL COMMENT 'metalake schema version info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'metalake 
current version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'metalake last 
version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'metalake 
deleted at',
+    PRIMARY KEY (metalake_id),
+    CONSTRAINT uk_mn_del UNIQUE (metalake_name, deleted_at)
+    ) ENGINE = InnoDB;
+
+
+CREATE TABLE IF NOT EXISTS `catalog_meta` (
+                                              `catalog_id` BIGINT(20) UNSIGNED 
NOT NULL COMMENT 'catalog id',
+    `catalog_name` VARCHAR(128) NOT NULL COMMENT 'catalog name',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `type` VARCHAR(64) NOT NULL COMMENT 'catalog type',
+    `provider` VARCHAR(64) NOT NULL COMMENT 'catalog provider',
+    `catalog_comment` VARCHAR(256) DEFAULT '' COMMENT 'catalog comment',
+    `properties` CLOB DEFAULT NULL COMMENT 'catalog properties',
+    `audit_info` CLOB NOT NULL COMMENT 'catalog audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'catalog current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'catalog last 
version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'catalog 
deleted at',
+    PRIMARY KEY (catalog_id),
+    KEY catalog_meta_idx_name_da (catalog_name, deleted_at),
+    CONSTRAINT uk_mid_cn_del UNIQUE (metalake_id, catalog_name, deleted_at)
+    ) ENGINE=InnoDB;
+
+
+CREATE TABLE IF NOT EXISTS `schema_meta` (
+                                             `schema_id` BIGINT(20) UNSIGNED 
NOT NULL COMMENT 'schema id',
+    `schema_name` VARCHAR(128) NOT NULL COMMENT 'schema name',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `catalog_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'catalog id',
+    `schema_comment` VARCHAR(256) DEFAULT '' COMMENT 'schema comment',
+    `properties` CLOB DEFAULT NULL COMMENT 'schema properties',
+    `audit_info` CLOB NOT NULL COMMENT 'schema audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'schema current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'schema last 
version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'schema 
deleted at',
+    PRIMARY KEY (schema_id),
+    CONSTRAINT uk_cid_sn_del UNIQUE (catalog_id, schema_name, deleted_at),
+    -- Aliases are used here, and indexes with the same name in H2 can only be 
created once.
+    KEY idx_smid (metalake_id),
+    KEY schema_meta_idx_name_da (schema_name, deleted_at)
+    ) ENGINE=InnoDB;
+
+CREATE TABLE IF NOT EXISTS `table_meta` (
+                                            `table_id` BIGINT(20) UNSIGNED NOT 
NULL COMMENT 'table id',
+    `table_name` VARCHAR(128) NOT NULL COMMENT 'table name',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `catalog_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'catalog id',
+    `schema_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'schema id',
+    `audit_info` CLOB NOT NULL COMMENT 'table audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'table current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'table last 
version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'table deleted 
at',
+    PRIMARY KEY (table_id),
+    CONSTRAINT uk_sid_tn_del UNIQUE (schema_id, table_name, deleted_at),
+    -- Aliases are used here, and indexes with the same name in H2 can only be 
created once.
+    KEY idx_tmid (metalake_id),
+    KEY idx_tcid (catalog_id),
+    KEY table_meta_idx_name_da (table_name, deleted_at)
+    ) ENGINE=InnoDB;
+
+
+CREATE TABLE IF NOT EXISTS `table_column_version_info` (
+                                                           `id` BIGINT(20) 
UNSIGNED NOT NULL AUTO_INCREMENT COMMENT 'auto increment id',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `catalog_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'catalog id',
+    `schema_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'schema id',
+    `table_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'table id',
+    `table_version` INT UNSIGNED NOT NULL COMMENT 'table version',
+    `column_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'column id',
+    `column_name` VARCHAR(128) NOT NULL COMMENT 'column name',
+    `column_position` INT UNSIGNED NOT NULL COMMENT 'column position, starting 
from 0',
+    `column_type` CLOB NOT NULL COMMENT 'column type',
+    `column_comment` VARCHAR(256) DEFAULT '' COMMENT 'column comment',
+    `column_nullable` TINYINT(1) NOT NULL DEFAULT 1 COMMENT 'column nullable, 
0 is not nullable, 1 is nullable',
+    `column_auto_increment` TINYINT(1) NOT NULL DEFAULT 0 COMMENT 'column auto 
increment, 0 is not auto increment, 1 is auto increment',
+    `column_default_value` CLOB DEFAULT NULL COMMENT 'column default value',
+    `column_op_type` TINYINT(1) NOT NULL COMMENT 'column operation type, 1 is 
create, 2 is update, 3 is delete',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'column 
deleted at',
+    `audit_info` CLOB NOT NULL COMMENT 'column audit info',
+    PRIMARY KEY (`id`),
+    UNIQUE KEY `uk_tid_ver_cid_del` (`table_id`, `table_version`, `column_id`, 
`deleted_at`),
+    KEY `idx_tcmid` (`metalake_id`),
+    KEY `idx_tccid` (`catalog_id`),
+    KEY `idx_tcsid` (`schema_id`)
+    ) ENGINE=InnoDB;
+
+
+CREATE TABLE IF NOT EXISTS `fileset_meta` (
+                                              `fileset_id` BIGINT(20) UNSIGNED 
NOT NULL COMMENT 'fileset id',
+    `fileset_name` VARCHAR(128) NOT NULL COMMENT 'fileset name',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `catalog_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'catalog id',
+    `schema_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'schema id',
+    `type` VARCHAR(64) NOT NULL COMMENT 'fileset type',
+    `audit_info` CLOB NOT NULL COMMENT 'fileset audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'fileset current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'fileset last 
version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'fileset 
deleted at',
+    PRIMARY KEY (fileset_id),
+    CONSTRAINT uk_sid_fn_del UNIQUE (schema_id, fileset_name, deleted_at),
+    -- Aliases are used here, and indexes with the same name in H2 can only be 
created once.
+    KEY idx_fmid (metalake_id),
+    KEY idx_fcid (catalog_id),
+    KEY fileset_meta_idx_name_da (fileset_name, deleted_at)
+    ) ENGINE=InnoDB;
+
+
+CREATE TABLE IF NOT EXISTS `fileset_version_info` (
+                                                      `id` BIGINT(20) UNSIGNED 
NOT NULL AUTO_INCREMENT COMMENT 'auto increment id',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `catalog_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'catalog id',
+    `schema_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'schema id',
+    `fileset_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'fileset id',
+    `version` INT UNSIGNED NOT NULL COMMENT 'fileset info version',
+    `fileset_comment` VARCHAR(256) DEFAULT '' COMMENT 'fileset comment',
+    `properties` CLOB DEFAULT NULL COMMENT 'fileset properties',
+    `storage_location_name` VARCHAR(128) NOT NULL DEFAULT 'default' COMMENT 
'fileset storage location name',
+    `storage_location` CLOB DEFAULT NULL COMMENT 'fileset storage location',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'fileset 
deleted at',
+    PRIMARY KEY (id),
+    CONSTRAINT uk_fid_ver_del UNIQUE (fileset_id, version, 
storage_location_name, deleted_at),
+    -- Aliases are used here, and indexes with the same name in H2 can only be 
created once.
+    KEY idx_fvmid (metalake_id),
+    KEY idx_fvcid (catalog_id)
+    ) ENGINE=InnoDB;
+
+CREATE TABLE IF NOT EXISTS `topic_meta` (
+                                            `topic_id` BIGINT(20) UNSIGNED NOT 
NULL COMMENT 'topic id',
+    `topic_name` VARCHAR(128) NOT NULL COMMENT 'topic name',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `catalog_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'catalog id',
+    `schema_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'schema id',
+    `comment` VARCHAR(256) DEFAULT '' COMMENT 'topic comment',
+    `properties` CLOB DEFAULT NULL COMMENT 'topic properties',
+    `audit_info` CLOB NOT NULL COMMENT 'topic audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'topic current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'topic last 
version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'topic deleted 
at',
+    PRIMARY KEY (topic_id),
+    CONSTRAINT uk_cid_tn_del UNIQUE (schema_id, topic_name, deleted_at),
+    -- Aliases are used here, and indexes with the same name in H2 can only be 
created once.
+    KEY idx_tvmid (metalake_id),
+    KEY idx_tvcid (catalog_id),
+    KEY topic_meta_idx_name_da (topic_name, deleted_at)
+    ) ENGINE=InnoDB;
+
+CREATE TABLE IF NOT EXISTS `user_meta` (
+                                           `user_id` BIGINT(20) UNSIGNED NOT 
NULL COMMENT 'user id',
+    `user_name` VARCHAR(128) NOT NULL COMMENT 'username',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `audit_info` CLOB NOT NULL COMMENT 'user audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'user current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'user last version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'user deleted 
at',
+    PRIMARY KEY (`user_id`),
+    CONSTRAINT `uk_mid_us_del` UNIQUE (`metalake_id`, `user_name`, 
`deleted_at`)
+    ) ENGINE=InnoDB;
+
+CREATE TABLE IF NOT EXISTS `role_meta` (
+                                           `role_id` BIGINT(20) UNSIGNED NOT 
NULL COMMENT 'role id',
+    `role_name` VARCHAR(128) NOT NULL COMMENT 'role name',
+    `metalake_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'metalake id',
+    `properties` CLOB DEFAULT NULL COMMENT 'schema properties',
+    `audit_info` CLOB NOT NULL COMMENT 'role audit info',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'role current 
version',
+    `last_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'role last version',
+    `deleted_at` BIGINT(20) UNSIGNED NOT NULL DEFAULT 0 COMMENT 'role deleted 
at',
+    PRIMARY KEY (`role_id`),
+    CONSTRAINT `uk_mid_rn_del` UNIQUE (`metalake_id`, `role_name`, 
`deleted_at`)
+    ) ENGINE=InnoDB;
+
+CREATE TABLE IF NOT EXISTS `role_meta_securable_object` (
+                                                            `id` BIGINT(20) 
UNSIGNED NOT NULL AUTO_INCREMENT COMMENT 'auto increment id',
+    `role_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'role id',
+    `metadata_object_id` BIGINT(20) UNSIGNED NOT NULL COMMENT 'securable 
object entity id',
+    `type`  VARCHAR(128) NOT NULL COMMENT 'securable object type',
+    `privilege_names` CLOB(81920) NOT NULL COMMENT 'securable object privilege 
names',
+    `privilege_conditions` CLOB(81920) NOT NULL COMMENT 'securable object 
privilege conditions',
+    `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'securable 
objectcurrent version',

Review Comment:
   There is a missing space in the comment text. The line reads 'securable 
objectcurrent version' but should read 'securable object current version' 
(missing space between 'object' and 'current').
   ```suggestion
       `current_version` INT UNSIGNED NOT NULL DEFAULT 1 COMMENT 'securable 
object current version',
   ```



##########
scripts/h2/schema-1.2.0-h2.sql:
##########
@@ -0,0 +1,451 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file--

Review Comment:
   There is a syntax error in the SQL comment. Line 3 has double dashes "--" 
where one comment block ends and another begins without proper spacing. This 
should be "-- or more contributor license agreements.  See the NOTICE file" 
with proper comment continuation, or the comment should be reformatted to avoid 
this issue.
   ```suggestion
   -- or more contributor license agreements.  See the NOTICE file
   ```



##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableMetaBaseSQLProvider.java:
##########
@@ -72,6 +75,57 @@ public String listTablePOsByTableIds(List<Long> tableIds) {
         + "</script>";
   }
 
+  public String listTablePOsByFullQualifiedName(
+      @Param("metalakeName") String metalakeName,
+      @Param("catalogName") String catalogName,
+      @Param("schemaName") String schemaName) {
+    return """
+        SELECT
+            sm.schema_id AS schemaId,
+            cm.catalog_id AS catalogId,
+            tm.table_id AS tableId,
+            tm.table_name AS tableName,
+            tm.metalake_id AS metalakeId,
+            tm.audit_info AS auditInfo,
+            tm.current_version AS currentVersion,
+            tm.last_version AS lastVersion,
+            tm.deleted_at AS deletedAt,
+            tvi.format AS format,
+            tvi.properties AS properties,
+            tvi.partitioning AS partitions,
+            tvi.sort_orders AS sortOrders,
+            tvi.distribution AS distribution,
+            tvi.indexes AS indexes,
+            tvi.comment AS comment
+        FROM
+            %s mm
+        INNER JOIN
+            %s cm ON mm.metalake_id = cm.metalake_id
+            AND cm.catalog_name = #{catalogName}
+            AND cm.deleted_at = 0
+        LEFT JOIN
+            %s sm ON cm.catalog_id = sm.catalog_id
+            AND sm.schema_name = #{schemaName}
+            AND sm.deleted_at = 0
+        LEFT JOIN
+            %s tm ON sm.schema_id = tm.schema_id
+            AND tm.deleted_at = 0
+        LEFT JOIN
+            %s tvi ON tm.table_id = tvi.table_id
+            AND tm.current_version = tvi.version
+            AND tvi.deleted_at = 0
+        WHERE
+            mm.metalake_name = #{metalakeName}
+            AND mm.deleted_at = 0;

Review Comment:
   The SQL query uses LEFT JOIN for the schema and table joins, which means it 
will return rows even when the schema or table doesn't exist (with NULL 
values). This could lead to incorrect behavior where the code attempts to 
process NULL entities. The join should be an INNER JOIN for sm (schema) since a 
table cannot exist without a schema. Only the table join (tm) should be LEFT 
JOIN if the intent is to list schemas even without tables.



-- 
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]

Reply via email to