This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-1.0
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.0 by this push:
new 031d7ec2d3 [#8408] improvement(sql): avoid returning [""] when no
roles exist in GroupMetaH2Provider (#8428)
031d7ec2d3 is described below
commit 031d7ec2d3e61cf321cdb58fd9c4143a3866d11d
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 4 15:49:29 2025 +0800
[#8408] improvement(sql): avoid returning [""] when no roles exist in
GroupMetaH2Provider (#8428)
### What changes were proposed in this pull request?
In `GroupMetaH2Provider.listExtendedGroupPOsByMetalakeId`, both
`roleNames` and `roleIds` now use:
- Added a `CASE` expression inside `GROUP_CONCAT` to filter out `NULL`
and empty strings, ensuring only valid role values are included i.e. not
mapped to `""`
- Used `COALESCE(GROUP_CONCAT(...), '')` to guarantee the aggregate
never evaluates to `NULL`
- Prevented the previous behavior where no roles resulted in `[""]`, by
ensuring that when no valid entries exist the result becomes `[]`.
- Preserved correct quoting of non-empty values (`"value"`) for
compatibility with JSON parsing expectations.
This guarantees clean JSON-like array strings.
### Why are the changes needed?
Previously, if no roles existed, the query returned `[""]`, which
misrepresents the absence of roles.
With this fix, such cases now correctly return `[]`.
This improves correctness and avoids downstream parsing issues.
Fix: #8408
### Does this PR introduce _any_ user-facing change?
Yes.
- **Before:** empty roles → `[""]`
- **After:** empty roles → `[]`
- Non-empty cases remain unchanged (e.g. `["Admin","User"]`).
- Invalid role values skipped (e.g. `["Admin","", null]` -->
`["Admin"]`)
### How was this patch tested?
- Verified with H2:
- Groups with multiple roles → quoted list.
- Groups with only empty/NULL roles → values skipped.
- Groups with no roles → `[]`.
- Added integration tests to confirm all scenarios.
Co-authored-by: Khawaja Abdullah Ansar <[email protected]>
---
.../mapper/provider/h2/GroupMetaH2Provider.java | 16 ++-
.../provider/h2/TestGroupMetaH2Provider.java | 144 +++++++++++++++++++++
2 files changed, 158 insertions(+), 2 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java
index e975131e09..45de807267 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/GroupMetaH2Provider.java
@@ -33,8 +33,20 @@ public class GroupMetaH2Provider extends
GroupMetaBaseSQLProvider {
+ " gt.audit_info as auditInfo,"
+ " gt.current_version as currentVersion, gt.last_version as
lastVersion,"
+ " gt.deleted_at as deletedAt,"
- + " '[' || GROUP_CONCAT('\"' || rot.role_name || '\"') || ']' as
roleNames,"
- + " '[' || GROUP_CONCAT('\"' || rot.role_id || '\"') || ']' as roleIds"
+ + " '[' || COALESCE(GROUP_CONCAT( "
+ + " CASE "
+ + " WHEN rot.role_name IS NOT NULL AND rot.role_name <> '' "
+ + " THEN '\"' || rot.role_name || '\"' "
+ + " ELSE NULL "
+ + " END "
+ + " ), '') || ']' as roleNames, "
+ + " '[' || COALESCE(GROUP_CONCAT( "
+ + " CASE "
+ + " WHEN rot.role_id IS NOT NULL "
+ + " THEN '\"' || rot.role_id || '\"' "
+ + " ELSE NULL "
+ + " END "
+ + " ), '') || ']' as roleIds "
+ " FROM "
+ GROUP_TABLE_NAME
+ " gt LEFT OUTER JOIN ("
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/mapper/provider/h2/TestGroupMetaH2Provider.java
b/core/src/test/java/org/apache/gravitino/storage/relational/mapper/provider/h2/TestGroupMetaH2Provider.java
new file mode 100644
index 0000000000..3d68c7f329
--- /dev/null
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/mapper/provider/h2/TestGroupMetaH2Provider.java
@@ -0,0 +1,144 @@
+/*
+ * 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.
+ */
+
+package org.apache.gravitino.storage.relational.mapper.provider.h2;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+class TestGroupMetaH2Provider {
+
+ private static final String DB_CONNECTION_URL =
"jdbc:h2:mem:test;MODE=MYSQL;DB_CLOSE_DELAY=-1";
+ private static final String DB_USER = "sa";
+ private static final String DB_PASSWORD = "";
+ private static final String COLUMN_LABEL_ROLE_NAMES = "roleNames";
+ private static final String COLUMN_LABEL_ROLE_IDS = "roleIds";
+ private static final String QUERY_PARAM_METALAKE_ID = "#{metalakeId}";
+
+ private static Connection connection;
+ private static Statement statement;
+ private static GroupMetaH2Provider groupMetaH2Provider;
+
+ @BeforeAll
+ static void setUp() throws SQLException {
+ connection = DriverManager.getConnection(DB_CONNECTION_URL, DB_USER,
DB_PASSWORD);
+ statement = connection.createStatement();
+ groupMetaH2Provider = new GroupMetaH2Provider();
+ createSchema(statement);
+ }
+
+ private static void createSchema(Statement statement) throws SQLException {
+ statement.execute(
+ "CREATE TABLE IF NOT EXISTS group_meta ("
+ + "group_id BIGINT, "
+ + "group_name VARCHAR(255), "
+ + "metalake_id BIGINT, "
+ + "audit_info VARCHAR(255), "
+ + "current_version BIGINT, "
+ + "last_version BIGINT, "
+ + "deleted_at BIGINT)");
+ statement.execute(
+ "CREATE TABLE IF NOT EXISTS role_meta ("
+ + "role_id BIGINT, "
+ + "role_name VARCHAR(255), "
+ + "deleted_at BIGINT)");
+ statement.execute(
+ "CREATE TABLE IF NOT EXISTS group_role_rel ("
+ + "group_id BIGINT, "
+ + "role_id BIGINT, "
+ + "deleted_at BIGINT)");
+ }
+
+ @Test
+ void testListExtendedGroupPOsByMetalakeIdWithoutRoles() throws SQLException {
+ statement.execute("INSERT INTO group_meta VALUES (1, 'g1', 1, 'audit', 0,
0, 0)");
+ String sql =
+ groupMetaH2Provider
+ .listExtendedGroupPOsByMetalakeId(1L)
+ .replace(QUERY_PARAM_METALAKE_ID, "1");
+ try (ResultSet rs = statement.executeQuery(sql)) {
+ rs.next();
+ assertEquals("[]", rs.getString(COLUMN_LABEL_ROLE_NAMES));
+ assertEquals("[]", rs.getString(COLUMN_LABEL_ROLE_IDS));
+ }
+ }
+
+ @Test
+ void testListExtendedGroupPOsByMetalakeIdWithRoles() throws SQLException {
+ statement.execute("INSERT INTO group_meta VALUES (2, 'g2', 2, 'audit2', 0,
0, 0)");
+ statement.execute("INSERT INTO role_meta VALUES (1, 'role1', 0)");
+ statement.execute("INSERT INTO role_meta VALUES (2, 'role2', 0)");
+ statement.execute("INSERT INTO group_role_rel VALUES (2, 1, 0)");
+ statement.execute("INSERT INTO group_role_rel VALUES (2, 2, 0)");
+
+ String sql =
+ groupMetaH2Provider
+ .listExtendedGroupPOsByMetalakeId(2L)
+ .replace(QUERY_PARAM_METALAKE_ID, "2");
+ try (ResultSet rs = statement.executeQuery(sql)) {
+ rs.next();
+ assertEquals("[\"role1\",\"role2\"]",
rs.getString(COLUMN_LABEL_ROLE_NAMES));
+ assertEquals("[\"1\",\"2\"]", rs.getString(COLUMN_LABEL_ROLE_IDS));
+ }
+ }
+
+ @Test
+ void testListExtendedGroupPOsByMetalakeIdWithInvalidRoles() throws
SQLException {
+ statement.execute("INSERT INTO group_meta VALUES (3, 'g3', 3, 'audit3', 0,
0, 0)");
+ statement.execute("INSERT INTO role_meta VALUES (3, 'role3', 0)");
+ statement.execute("INSERT INTO role_meta VALUES (4, '', 0)");
+ statement.execute("INSERT INTO role_meta VALUES (5, null, 0)");
+ statement.execute("INSERT INTO role_meta VALUES (null, 'role6', 0)");
+ statement.execute("INSERT INTO group_role_rel VALUES (3, 3, 0)");
+ statement.execute("INSERT INTO group_role_rel VALUES (3, 4, 0)");
+ statement.execute("INSERT INTO group_role_rel VALUES (3, 5, 0)");
+ statement.execute("INSERT INTO group_role_rel VALUES (3, null, 0)");
+
+ String sql =
+ groupMetaH2Provider
+ .listExtendedGroupPOsByMetalakeId(3L)
+ .replace(QUERY_PARAM_METALAKE_ID, "3");
+ try (ResultSet rs = statement.executeQuery(sql)) {
+ rs.next();
+ assertEquals("[\"role3\"]", rs.getString(COLUMN_LABEL_ROLE_NAMES));
+ assertEquals("[\"3\",\"4\",\"5\"]", rs.getString(COLUMN_LABEL_ROLE_IDS));
+ }
+ }
+
+ @AfterAll
+ static void tearDown() throws SQLException {
+ dropSchema(statement);
+ statement.close();
+ connection.close();
+ }
+
+ private static void dropSchema(Statement statement) throws SQLException {
+ statement.execute("DROP TABLE IF EXISTS group_role_rel");
+ statement.execute("DROP TABLE IF EXISTS role_meta");
+ statement.execute("DROP TABLE IF EXISTS group_meta");
+ }
+}
