dimas-b commented on code in PR #2508:
URL: https://github.com/apache/polaris/pull/2508#discussion_r2323213597
##########
polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisBaseEntity.java:
##########
@@ -339,10 +339,7 @@ public boolean equals(Object o) {
PolarisBaseEntity that = (PolarisBaseEntity) o;
return subTypeCode == that.subTypeCode
&& createTimestamp == that.createTimestamp
- && dropTimestamp == that.dropTimestamp
- && purgeTimestamp == that.purgeTimestamp
Review Comment:
Is this change related to the stated purpose of this PR?
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java:
##########
@@ -456,7 +457,12 @@ public List<PolarisBaseEntity> lookupEntities(
if (entityIds == null || entityIds.isEmpty()) return new ArrayList<>();
PreparedQuery query =
QueryGenerator.generateSelectQueryWithEntityIds(realmId, entityIds);
try {
- return datasourceOperations.executeSelect(query, new ModelEntity());
+ Map<PolarisEntityId, PolarisBaseEntity> idMap =
+ datasourceOperations.executeSelect(query, new ModelEntity()).stream()
+ .collect(
+ Collectors.toMap(
+ e -> new PolarisEntityId(e.getCatalogId(), e.getId()),
Function.identity()));
+ return entityIds.stream().map(idMap::get).collect(Collectors.toList());
Review Comment:
Since we're obviously enforcing the order of the returned values to match
the order of requested IDs, could you add a test for that?
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java:
##########
@@ -1531,14 +1532,42 @@ private void revokeGrantRecord(
: new EntityResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null);
}
+ @Nonnull
+ @Override
+ public EntitiesResult loadEntities(
+ @Nonnull PolarisCallContext callCtx,
+ @Nonnull List<EntityNameLookupRecord> entityLookupRecords) {
+ BasePersistence ms = callCtx.getMetaStore();
+ List<PolarisBaseEntity> entities =
+ ms.lookupEntities(
+ callCtx,
+ entityLookupRecords.stream()
+ .map(r -> new PolarisEntityId(r.getCatalogId(), r.getId()))
+ .collect(Collectors.toList()));
+ // mimic the behavior of loadEntity above, return null if not found or
type mismatch
+ List<PolarisBaseEntity> ret =
+ IntStream.range(0, entityLookupRecords.size())
+ .mapToObj(
+ i -> {
+ if (entities.get(i) != null
+ &&
!entities.get(i).getType().equals(entityLookupRecords.get(i).getType())) {
+ return null;
+ } else {
+ return entities.get(i);
Review Comment:
Rather than assuming fast access by position in the `entities` list, would
it be simpler and more generalizable to iterate `entities` and
`entityLookupRecords` in parallel?
--
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]
