Add entitySubType param to BasePersistence.listEntities

[XN137]

BasePersistence.listEntities has 3 variants:

Page<EntityNameLookupRecord> listEntities(..., PageToken);

Page<EntityNameLookupRecord> listEntities(..., Predicate<PolarisBaseEntity>, PageToken)

<T> Page<T> listEntities(..., Predicate<PolarisBaseEntity>, Function<PolarisBaseEntity, T>, PageToken);

the 1st method exists to only return the subset of entity properties required to build an EntityNameLookupRecord.

the 3rd method supports a predicate and transformer function on the underlying PolarisBaseEntity, which means it has to load all entity properties.

the 2nd method is weird as it supports a full Predicate<PolarisBaseEntity>, which means it has to load all entity properties under the hood for filtering but then throws most of them away to return a EntityNameLookupRecord.
this explains why the implementations of the 2nd method simply forward to the 3rd method usually.
any performance benefits of returning a EntityNameLookupRecord are lost.

as it turns out the 2nd method is only used, because methods 1 and 3 dont support passing a PolarisEntitySubType parameter to filter down the retrieved data.
Note that the sub type property is available from both the PolarisBaseEntity as well as the EntityNameLookupRecord.

By adding this parameter, the 2nd method can go away completely.
we can even push down the sub type filtering into the queries of some of our persistence implementations.
other existing implementations are free to decide whether they want to push it down as well or filter on the query results in memory.

note that since we have no TransactionalPersistence implementation in the codebase that provides an optimized variant of method 1 we can have a default method in the interface that forwards to method 3.

[XN137]

note that due to this change calls to PolarisMetaStoreManager.listEntities like here:

polaris/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

Lines 2574 to 2591 in 2f985ab

	private Page<TableIdentifier> listTableLike(
	PolarisEntitySubType subType, Namespace namespace, PageToken pageToken) {
	PolarisResolvedPathWrapper resolvedEntities = resolvedEntityView.getResolvedPath(namespace);
	if (resolvedEntities == null) {
	// Illegal state because the namespace should've already been in the static resolution set.
	throw new IllegalStateException(
	String.format("Failed to fetch resolved namespace '%s'", namespace));
	}
	List<PolarisEntity> catalogPath = resolvedEntities.getRawFullPath();
	ListEntitiesResult listResult =
	getMetaStoreManager()
	.listEntities(
	getCurrentPolarisContext(),
	PolarisEntity.toCoreList(catalogPath),
	PolarisEntityType.TABLE_LIKE,
	subType,
	pageToken);

no longer have to load the full PolarisBaseEntity to apply the Predicate<PolarisBaseEntity> and thus they can actually take advantage of optimized implementations of Page<EntityNameLookupRecord> listEntities in BasePersistence that load only the required properties.

[adutra]

Awesome improvement!

[dennishuo]

LGTM, thanks!

I also would like to adjust the transformer/filter version of listEntities to better reflect it as less of an overloaded method and more of a totally distinct method, but perhaps that fits in better in your other PR #2290

[dennishuo]

To help us remember as well, can we add and update the underlying CONSTRAINT clause to make the implicit index INCLUDE the subTypeCode to this TODO?

[XN137]

i'll take a stab at this as an immediate follow-up or file it as a separate issue where i will take note of this additional "index coverage" requirement

[XN137]

filed the ticket: #2352

[dimas-b]

Related dev ML discussion: https://lists.apache.org/thread/kxyqp287zmxmorod7l205ojkwfqgrhkv