This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new c99fc84341 [#11048] feat(core): batch SQL for JDBCBackend.batchGet
GROUP entity (#11046)
c99fc84341 is described below
commit c99fc84341bca82936f24b94ef4e3dce0f64bb15
Author: Bharath Krishna <[email protected]>
AuthorDate: Mon May 11 23:13:48 2026 -0700
[#11048] feat(core): batch SQL for JDBCBackend.batchGet GROUP entity
(#11046)
### What changes were proposed in this pull request?
Replace the per-identifier loop for `GROUP` in `JDBCBackend.batchGet`
with a single batched SQL SELECT that returns groups and their roles in
one round-trip.
- New `listExtendedGroupPOsByMetalakeIdAndNames` in
`GroupMetaBaseSQLProvider` (with H2 / PostgreSQL overrides) — mirrors
the existing `listExtendedGroupPOsByMetalakeId` JOIN + `JSON_ARRAYAGG`
template, filtered by `group_name IN (...)` via MyBatis
`<script><foreach>` (portable IN-clause, soft-delete aware, uses indexed
`(metalake_id, group_name)`).
- New `GroupMetaService.batchGetGroupByIdentifier`: cross-id
same-namespace check, single transaction via
`SessionUtils.doWithCommitAndFetchResult`, reuses `ExtendedGroupPO` +
`POConverters.fromExtendedGroupPO` to attach role names/ids — no extra
role round-trip.
- `JDBCBackend.batchGet` `GROUP` case dispatches to the new path.
### Why are the changes needed?
Reduces N round-trips to a single JOIN query on the GROUP batch path.
Fix: #11048
### Does this PR introduce any user-facing change?
No. Behavior contract is unchanged and consistent with sibling batches:
missing groups skipped (no exception), duplicates deduped, order not
preserved, all identifiers must share a namespace.
### How was this patch tested?
- New unit test `testBatchGetGroups` covers happy-path multi-group
retrieval and role attachment.
- Existing `testBatchGetGroupsPartialResults` covers missing groups.
- `./gradlew :core:test --tests "...TestJDBCBackendBatchGet" --tests
"...TestGroupMetaService" --tests "...TestRoleMetaService" -PskipITs` →
pass.
- `./gradlew :core:spotlessApply` → clean.
---
.../gravitino/storage/relational/JDBCBackend.java | 12 +--
.../storage/relational/mapper/GroupMetaMapper.java | 6 ++
.../mapper/GroupMetaSQLProviderFactory.java | 6 ++
.../provider/base/GroupMetaBaseSQLProvider.java | 36 ++++++++
.../mapper/provider/h2/GroupMetaH2Provider.java | 48 +++++++++++
.../postgresql/GroupMetaPostgreSQLProvider.java | 36 ++++++++
.../relational/service/GroupMetaService.java | 37 ++++++++
.../relational/TestJDBCBackendBatchGet.java | 98 ++++++++++++++++++++++
8 files changed, 269 insertions(+), 10 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
index b0f11c150e..22c7c98330 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java
@@ -351,7 +351,7 @@ public class JDBCBackend implements RelationalBackend {
return (List<E>)
JobTemplateMetaService.getInstance().batchGetJobTemplateByIdentifier(identifiers);
case USER:
- // TODO: Add true batch SQL operations for users, groups, roles, and
views
+ // TODO: Add true batch SQL operations for users, roles, and views
List<E> users = Lists.newArrayList();
for (NameIdentifier identifier : identifiers) {
try {
@@ -362,15 +362,7 @@ public class JDBCBackend implements RelationalBackend {
}
return users;
case GROUP:
- List<E> groups = Lists.newArrayList();
- for (NameIdentifier identifier : identifiers) {
- try {
- groups.add((E)
GroupMetaService.getInstance().getGroupByIdentifier(identifier));
- } catch (NoSuchEntityException e) {
- LOG.debug("Skipping missing group during batch get: {}",
identifier.name());
- }
- }
- return groups;
+ return (List<E>)
GroupMetaService.getInstance().batchGetGroupByIdentifier(identifiers);
case ROLE:
List<E> roles = Lists.newArrayList();
for (NameIdentifier identifier : identifiers) {
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java
index 775eb73973..6b0ac15da9 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaMapper.java
@@ -53,6 +53,12 @@ public interface GroupMetaMapper {
GroupPO selectGroupMetaByMetalakeIdAndName(
@Param("metalakeId") Long metalakeId, @Param("groupName") String name);
+ @SelectProvider(
+ type = GroupMetaSQLProviderFactory.class,
+ method = "listExtendedGroupPOsByMetalakeIdAndNames")
+ List<ExtendedGroupPO> listExtendedGroupPOsByMetalakeIdAndNames(
+ @Param("metalakeId") Long metalakeId, @Param("groupNames") List<String>
groupNames);
+
@SelectProvider(type = GroupMetaSQLProviderFactory.class, method =
"listGroupPOsByMetalake")
List<GroupPO> listGroupPOsByMetalake(@Param("metalakeName") String
metalakeName);
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java
index 729a6a1d5b..a846b6545f 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/GroupMetaSQLProviderFactory.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.storage.relational.mapper;
import com.google.common.collect.ImmutableMap;
+import java.util.List;
import java.util.Map;
import org.apache.gravitino.storage.relational.JDBCBackend.JDBCBackendType;
import
org.apache.gravitino.storage.relational.mapper.provider.base.GroupMetaBaseSQLProvider;
@@ -58,6 +59,11 @@ public class GroupMetaSQLProviderFactory {
return getProvider().selectGroupMetaByMetalakeIdAndName(metalakeId, name);
}
+ public static String listExtendedGroupPOsByMetalakeIdAndNames(
+ @Param("metalakeId") Long metalakeId, @Param("groupNames") List<String>
groupNames) {
+ return getProvider().listExtendedGroupPOsByMetalakeIdAndNames(metalakeId,
groupNames);
+ }
+
public static String insertGroupMeta(@Param("groupMeta") GroupPO groupPO) {
return getProvider().insertGroupMeta(groupPO);
}
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java
index 353a6cf90a..51523d8e6f 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/GroupMetaBaseSQLProvider.java
@@ -22,6 +22,7 @@ import static
org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GRO
import static
org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME;
import static
org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.ROLE_TABLE_NAME;
+import java.util.List;
import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper;
import org.apache.gravitino.storage.relational.po.GroupPO;
import org.apache.ibatis.annotations.Param;
@@ -86,6 +87,41 @@ public class GroupMetaBaseSQLProvider {
+ " AND deleted_at = 0";
}
+ public String listExtendedGroupPOsByMetalakeIdAndNames(
+ @Param("metalakeId") Long metalakeId, @Param("groupNames") List<String>
groupNames) {
+ return "<script>"
+ + "SELECT gt.group_id as groupId, gt.group_name as groupName,"
+ + " gt.metalake_id as metalakeId,"
+ + " gt.audit_info as auditInfo,"
+ + " gt.current_version as currentVersion, gt.last_version as
lastVersion,"
+ + " gt.deleted_at as deletedAt,"
+ + " JSON_ARRAYAGG(rot.role_name) as roleNames,"
+ + " JSON_ARRAYAGG(rot.role_id) as roleIds"
+ + " FROM "
+ + GROUP_TABLE_NAME
+ + " gt LEFT OUTER JOIN ("
+ + " SELECT * FROM "
+ + GROUP_ROLE_RELATION_TABLE_NAME
+ + " WHERE deleted_at = 0)"
+ + " AS rt ON rt.group_id = gt.group_id"
+ + " LEFT OUTER JOIN ("
+ + " SELECT * FROM "
+ + ROLE_TABLE_NAME
+ + " WHERE deleted_at = 0)"
+ + " AS rot ON rot.role_id = rt.role_id"
+ + " WHERE"
+ + " gt.deleted_at = 0 AND"
+ + " gt.metalake_id = #{metalakeId}"
+ + " AND gt.group_name IN ("
+ + "<foreach collection='groupNames' item='groupName' separator=','>"
+ + "#{groupName}"
+ + "</foreach>"
+ + " )"
+ + " GROUP BY gt.group_id, gt.group_name, gt.metalake_id,
gt.audit_info,"
+ + " gt.current_version, gt.last_version, gt.deleted_at"
+ + "</script>";
+ }
+
public String insertGroupMeta(@Param("groupMeta") GroupPO groupPO) {
return "INSERT INTO "
+ GROUP_TABLE_NAME
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 e2d2549d8a..2cec58f6c6 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
@@ -22,6 +22,7 @@ import static
org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GRO
import static
org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME;
import static
org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.ROLE_TABLE_NAME;
+import java.util.List;
import
org.apache.gravitino.storage.relational.mapper.provider.base.GroupMetaBaseSQLProvider;
import org.apache.ibatis.annotations.Param;
@@ -64,4 +65,51 @@ public class GroupMetaH2Provider extends
GroupMetaBaseSQLProvider {
+ " gt.metalake_id = #{metalakeId}"
+ " GROUP BY gt.group_id";
}
+
+ @Override
+ public String listExtendedGroupPOsByMetalakeIdAndNames(
+ @Param("metalakeId") Long metalakeId, @Param("groupNames") List<String>
groupNames) {
+ return "<script>"
+ + "SELECT gt.group_id as groupId, gt.group_name as groupName,"
+ + " gt.metalake_id as metalakeId,"
+ + " gt.audit_info as auditInfo,"
+ + " gt.current_version as currentVersion, gt.last_version as
lastVersion,"
+ + " gt.deleted_at as deletedAt,"
+ + " '[' || 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 ("
+ + " SELECT * FROM "
+ + GROUP_ROLE_RELATION_TABLE_NAME
+ + " WHERE deleted_at = 0)"
+ + " AS rt ON rt.group_id = gt.group_id"
+ + " LEFT OUTER JOIN ("
+ + " SELECT * FROM "
+ + ROLE_TABLE_NAME
+ + " WHERE deleted_at = 0)"
+ + " AS rot ON rot.role_id = rt.role_id"
+ + " WHERE"
+ + " gt.deleted_at = 0 AND"
+ + " gt.metalake_id = #{metalakeId}"
+ + " AND gt.group_name IN ("
+ + "<foreach collection='groupNames' item='groupName' separator=','>"
+ + "#{groupName}"
+ + "</foreach>"
+ + " )"
+ + " GROUP BY gt.group_id"
+ + "</script>";
+ }
}
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java
index b11c7972eb..c70dee00f9 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/GroupMetaPostgreSQLProvider.java
@@ -22,6 +22,7 @@ import static
org.apache.gravitino.storage.relational.mapper.GroupMetaMapper.GRO
import static
org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.GROUP_ROLE_RELATION_TABLE_NAME;
import static
org.apache.gravitino.storage.relational.mapper.RoleMetaMapper.ROLE_TABLE_NAME;
+import java.util.List;
import
org.apache.gravitino.storage.relational.mapper.provider.base.GroupMetaBaseSQLProvider;
import org.apache.gravitino.storage.relational.po.GroupPO;
import org.apache.ibatis.annotations.Param;
@@ -95,6 +96,41 @@ public class GroupMetaPostgreSQLProvider extends
GroupMetaBaseSQLProvider {
+ " GROUP BY gt.group_id";
}
+ @Override
+ public String listExtendedGroupPOsByMetalakeIdAndNames(
+ @Param("metalakeId") Long metalakeId, @Param("groupNames") List<String>
groupNames) {
+ return "<script>"
+ + "SELECT gt.group_id as groupId, gt.group_name as groupName,"
+ + " gt.metalake_id as metalakeId,"
+ + " gt.audit_info as auditInfo,"
+ + " gt.current_version as currentVersion, gt.last_version as
lastVersion,"
+ + " gt.deleted_at as deletedAt,"
+ + " JSON_AGG(rot.role_name) as roleNames,"
+ + " JSON_AGG(rot.role_id) as roleIds"
+ + " FROM "
+ + GROUP_TABLE_NAME
+ + " gt LEFT OUTER JOIN ("
+ + " SELECT * FROM "
+ + GROUP_ROLE_RELATION_TABLE_NAME
+ + " WHERE deleted_at = 0)"
+ + " AS rt ON rt.group_id = gt.group_id"
+ + " LEFT OUTER JOIN ("
+ + " SELECT * FROM "
+ + ROLE_TABLE_NAME
+ + " WHERE deleted_at = 0)"
+ + " AS rot ON rot.role_id = rt.role_id"
+ + " WHERE "
+ + " gt.deleted_at = 0 AND"
+ + " gt.metalake_id = #{metalakeId}"
+ + " AND gt.group_name IN ("
+ + "<foreach collection='groupNames' item='groupName' separator=','>"
+ + "#{groupName}"
+ + "</foreach>"
+ + " )"
+ + " GROUP BY gt.group_id"
+ + "</script>";
+ }
+
@Override
public String deleteGroupMetasByLegacyTimeline(
@Param("legacyTimeline") Long legacyTimeline, @Param("limit") int limit)
{
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
index 93399373b2..7a7914f223 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/service/GroupMetaService.java
@@ -109,6 +109,43 @@ public class GroupMetaService {
return POConverters.fromGroupPO(groupPO, rolePOs, identifier.namespace());
}
+ @Monitored(
+ metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
+ baseMetricName = "batchGetGroupByIdentifier")
+ public List<GroupEntity> batchGetGroupByIdentifier(List<NameIdentifier>
identifiers) {
+ if (identifiers == null || identifiers.isEmpty()) {
+ return Collections.emptyList();
+ }
+
+ NameIdentifier firstIdent = identifiers.get(0);
+ Namespace namespace = firstIdent.namespace();
+ String metalake = NameIdentifierUtil.getMetalake(firstIdent);
+
+ for (NameIdentifier identifier : identifiers) {
+ AuthorizationUtils.checkGroup(identifier);
+ Preconditions.checkArgument(
+ identifier.namespace().equals(namespace),
+ "All group identifiers must belong to the same namespace, expected
%s but got %s",
+ namespace,
+ identifier.namespace());
+ }
+
+ long metalakeId =
+ EntityIdService.getEntityId(NameIdentifier.of(metalake),
Entity.EntityType.METALAKE);
+ List<String> groupNames =
+
identifiers.stream().map(NameIdentifier::name).collect(Collectors.toList());
+
+ return SessionUtils.doWithCommitAndFetchResult(
+ GroupMetaMapper.class,
+ mapper -> {
+ List<ExtendedGroupPO> extendedPOs =
+ mapper.listExtendedGroupPOsByMetalakeIdAndNames(metalakeId,
groupNames);
+ return extendedPOs.stream()
+ .map(po -> POConverters.fromExtendedGroupPO(po, namespace))
+ .collect(Collectors.toList());
+ });
+ }
+
@Monitored(
metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "listGroupsByRoleIdent")
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackendBatchGet.java
b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackendBatchGet.java
index 327ba0fcb4..a7410990b3 100644
---
a/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackendBatchGet.java
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/TestJDBCBackendBatchGet.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.storage.relational;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import java.io.IOException;
import java.util.HashMap;
import java.util.List;
@@ -852,6 +853,103 @@ public class TestJDBCBackendBatchGet extends
TestJDBCBackend {
Assertions.assertEquals("catalog_single", result.get(0).name());
}
+ @TestTemplate
+ public void testBatchGetGroups() throws IOException {
+ String metalakeName = "metalake_for_group_batch";
+ createAndInsertMakeLake(metalakeName);
+
+ // Create a catalog and two roles so we can attach roles to one of the
groups
+ CatalogEntity catalog =
+ createCatalog(
+ RandomIdGenerator.INSTANCE.nextId(),
+ NamespaceUtil.ofCatalog(metalakeName),
+ "catalog_for_group_batch",
+ AUDIT_INFO);
+ backend.insert(catalog, false);
+
+ RoleEntity roleA =
+ createRoleEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ NamespaceUtil.ofRole(metalakeName),
+ "role_a",
+ AUDIT_INFO,
+ "catalog_for_group_batch");
+ RoleEntity roleB =
+ createRoleEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ NamespaceUtil.ofRole(metalakeName),
+ "role_b",
+ AUDIT_INFO,
+ "catalog_for_group_batch");
+ backend.insert(roleA, false);
+ backend.insert(roleB, false);
+
+ // group1: no roles. group2: two roles. group3: no roles.
+ GroupEntity group1 =
+ createGroupEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ NamespaceUtil.ofGroup(metalakeName),
+ "group1",
+ AUDIT_INFO,
+ Lists.newArrayList(),
+ Lists.newArrayList());
+ GroupEntity group2 =
+ createGroupEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ NamespaceUtil.ofGroup(metalakeName),
+ "group2",
+ AUDIT_INFO,
+ Lists.newArrayList(roleA.name(), roleB.name()),
+ Lists.newArrayList(roleA.id(), roleB.id()));
+ GroupEntity group3 =
+ createGroupEntity(
+ RandomIdGenerator.INSTANCE.nextId(),
+ NamespaceUtil.ofGroup(metalakeName),
+ "group3",
+ AUDIT_INFO,
+ Lists.newArrayList(),
+ Lists.newArrayList());
+ backend.insert(group1, false);
+ backend.insert(group2, false);
+ backend.insert(group3, false);
+
+ List<NameIdentifier> identifiers =
+ Lists.newArrayList(
+ group1.nameIdentifier(), group2.nameIdentifier(),
group3.nameIdentifier());
+
+ List<GroupEntity> result = backend.batchGet(identifiers,
Entity.EntityType.GROUP);
+
+ Assertions.assertEquals(3, result.size());
+ Map<String, GroupEntity> resultMap =
+ result.stream().collect(Collectors.toMap(GroupEntity::name, g -> g));
+
+ GroupEntity retrieved1 = resultMap.get("group1");
+ Assertions.assertNotNull(retrieved1);
+ Assertions.assertEquals(group1.id(), retrieved1.id());
+ Assertions.assertEquals(group1.namespace(), retrieved1.namespace());
+ Assertions.assertTrue(
+ retrieved1.roleIds() == null || retrieved1.roleIds().isEmpty(),
+ "group1 should have no roles");
+
+ GroupEntity retrieved2 = resultMap.get("group2");
+ Assertions.assertNotNull(retrieved2);
+ Assertions.assertEquals(group2.id(), retrieved2.id());
+ Assertions.assertEquals(group2.namespace(), retrieved2.namespace());
+ Assertions.assertNotNull(retrieved2.roleIds());
+ Assertions.assertEquals(2, retrieved2.roleIds().size());
+ Assertions.assertEquals(
+ Sets.newHashSet(roleA.id(), roleB.id()),
Sets.newHashSet(retrieved2.roleIds()));
+ Assertions.assertEquals(
+ Sets.newHashSet(roleA.name(), roleB.name()),
Sets.newHashSet(retrieved2.roleNames()));
+
+ GroupEntity retrieved3 = resultMap.get("group3");
+ Assertions.assertNotNull(retrieved3);
+ Assertions.assertEquals(group3.id(), retrieved3.id());
+ Assertions.assertTrue(
+ retrieved3.roleIds() == null || retrieved3.roleIds().isEmpty(),
+ "group3 should have no roles");
+ }
+
@TestTemplate
public void testBatchGetGroupsPartialResults() throws IOException {
String metalakeName = "metalake_for_group_partial";
