yuqi1129 commented on code in PR #14194:
URL: https://github.com/apache/gravitino/pull/14194#discussion_r4388191343
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/TableColumnBaseSQLProvider.java:
##########
@@ -141,6 +141,22 @@ public String selectColumnIdByTableIdAndName(
+ " ORDER BY c.table_version DESC, c.id DESC LIMIT 1";
}
+ /** Returns the SQL for a batched active-column lookup. */
+ public String selectColumnIdsByTableIdAndNames(Long tableId, List<String>
columnNames) {
Review Comment:
[Important] Could we add backend-parametrized regression tests for both new
batch queries? Currently their only real-database coverage comes indirectly
from the H2-only TestPolicyManager test with one table and one column. Neither
TestTableColumnMetaService nor TestTagMetaService invokes the new batch
methods, so the MySQL/PostgreSQL suites can pass without exercising this SQL.
Please add @TestTemplate cases in those storage-service test classes for the
>500 batch boundary, column rename/delete/same-name replacement, missing
columns, and attribution of multi-valued tag assignments to different source
objects. I temporarily exercised the column lifecycle and 5,000-column cases
against H2 and they passed; preserving those assertions in the PR would protect
this separate query path and let the backend lanes validate it.
##########
core/src/main/java/org/apache/gravitino/policy/ObjectPolicyResolver.java:
##########
@@ -104,18 +99,52 @@ public Match[] resolveMatches(String metalake,
MetadataObject metadataObject) {
return new Match[0];
}
- Map<String, ResolvedTag> tagsByName =
- Arrays.stream(effectiveTags)
- .collect(
- Collectors.toMap(
- resolvedTag -> resolvedTag.tag().name(),
- resolvedTag -> resolvedTag,
- (left, right) -> left,
- LinkedHashMap::new));
List<NameIdentifier> tagIdentifiers =
- tagsByName.keySet().stream()
+ Arrays.stream(effectiveTags)
+ .map(resolvedTag -> resolvedTag.tag().name())
+ .distinct()
.map(tagName -> NameIdentifierUtil.ofTag(metalake, tagName))
.collect(Collectors.toList());
+ return selectMatches(effectiveTags, loadRelations(tagIdentifiers,
metadataObject));
+ }
+
+ /**
+ * Resolves effective policies for multiple objects using shared tag and
policy reads.
+ *
+ * @param metalake the metalake name
+ * @param metadataObjects the objects to resolve
+ * @return policies keyed by requested object
+ */
+ public Map<MetadataObject, PolicyEntity[]> resolve(
+ String metalake, List<MetadataObject> metadataObjects) {
+ if (metadataObjects.isEmpty()) {
+ return new LinkedHashMap<>();
+ }
+ Map<MetadataObject, ResolvedTag[]> tagsByObject =
+ effectiveTagResolver.resolveWithSources(metalake, metadataObjects);
+ Map<String, NameIdentifier> tagIdentifiers = new LinkedHashMap<>();
+ tagsByObject
+ .values()
+ .forEach(
+ tags -> {
+ for (ResolvedTag resolvedTag : tags) {
+ TagEntity tag = resolvedTag.tag();
+ tagIdentifiers.putIfAbsent(
+ tag.name(), NameIdentifierUtil.ofTag(metalake,
tag.name()));
+ }
+ });
+ List<RelationalEntity<?>> relations =
+ tagIdentifiers.isEmpty()
+ ? Collections.emptyList()
+ : loadRelations(new ArrayList<>(tagIdentifiers.values()),
metadataObjects);
+ Map<MetadataObject, PolicyEntity[]> result = new LinkedHashMap<>();
+ tagsByObject.forEach(
+ (object, tags) -> result.put(object, policies(selectMatches(tags,
relations))));
Review Comment:
[Nit, non-blocking] Each object still scans the complete union of policy
relations through selectMatches(tags, relations), including relations belonging
only to other objects. With distinct tags per column, matching work becomes
O(number of objects * total relations). A local counting probe with 1,000
columns and one distinct tag/relation per column recorded 1,000,000 relation
visits, while returning the expected policies.
Consider indexing the shared relations by source tag name once, then looking
up only the relations for each object's effective tags. The database-query
reduction is still useful and the current results are correct, so this can be
deferred as a follow-up optimization.
--
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]
