Remove unused LIST_PAGINATION_ENABLED feature configuration#2296
Remove unused LIST_PAGINATION_ENABLED feature configuration#2296adutra wants to merge 1 commit intoapache:mainfrom
Conversation
|
Why is this unused? |
I have no idea. It was introduced in #1528, that's all I know. |
|
I think that this should have been used in #1938; does pagination now ignore this flag? |
|
Adding if/else on the paginated call paths looks a bit of an overkill to me. As currently implemented, IIRC, pagination is controlled by the client. If the client does not request a call to be paginated, the server will work as before #1938. IMHO, the risk of breaking clients with current pagination code is low. |
|
We have feature flags for tons of things controlled by the client -- like table location overlap, generic tables, drop-with-purge, policies, etc. This flag was put in place specifically to guard against regressions caused by the new code path introduced by pagination. |
|
Fair point. I'll look into that presently. |
|
Moving this to draft while @dimas-b looks into this issue. |
The enforcement of the LIST_PAGINATION_ENABLED flag was missed in apache#1938. This change make the flag effective as discussed in apache#2296. Note: this causes a change in the default Polaris behaviour (no pagination by default) with respect to the previous state of `main`. However, there is no behaviour change with respect to 1.0.0 or 1.0.1 as previous releases did not have apache#1938.
|
Following up in #2401 |
|
Closing then, thanks @dimas-b ! |
The enforcement of the LIST_PAGINATION_ENABLED flag was missed in #1938. This change make the flag effective as discussed in #2296. Note: this causes a change in the default Polaris behaviour (no pagination by default) with respect to the previous state of `main`. However, there is no behaviour change with respect to 1.0.0 or 1.0.1 as previous releases did not have #1938.
The enforcement of the LIST_PAGINATION_ENABLED flag was missed in apache#1938. This change make the flag effective as discussed in apache#2296. Note: this causes a change in the default Polaris behaviour (no pagination by default) with respect to the previous state of `main`. However, there is no behaviour change with respect to 1.0.0 or 1.0.1 as previous releases did not have apache#1938.
* feat: enforce LIST_PAGINATION_ENABLED The enforcement of the LIST_PAGINATION_ENABLED flag was missed in #1938. This change make the flag effective as discussed in #2296. Note: this causes a change in the default Polaris behaviour (no pagination by default) with respect to the previous state of `main`. However, there is no behaviour change with respect to 1.0.0 or 1.0.1 as previous releases did not have #1938.
* feat: enforce LIST_PAGINATION_ENABLED (apache#2401) * feat: enforce LIST_PAGINATION_ENABLED The enforcement of the LIST_PAGINATION_ENABLED flag was missed in apache#1938. This change make the flag effective as discussed in apache#2296. Note: this causes a change in the default Polaris behaviour (no pagination by default) with respect to the previous state of `main`. However, there is no behaviour change with respect to 1.0.0 or 1.0.1 as previous releases did not have apache#1938. * Add PolarisMetaStoreManager.loadEntities (apache#2290) * Add PolarisMetaStoreManager.loadEntities currently `PolarisMetaStoreManager.listEntities` only exposes a limited subset of the underlying `BasePersistence.listEntities` functionality. most of the callers have to post-process the `EntityNameLookupRecord` of `ListEntitiesResult` and call `PolarisMetaStoreManager.loadEntity` on the individual items sequentually to transform and filter them. this is bad for the following reasons: - suboptimal performance as we run N+1 queries to basically load every entity twice from the persistence backend - suffering from race-conditions when entities get dropped between the `listEntities` and `loadEntity` call - a lot of repeated code in all the callers (of which only some are dealing with the race-condition by filtering out null values) as a solution we add `PolarisMetaStoreManager.loadEntities` that takes advantage of the already existing `BasePersistence` methods. we rename one of the `listEntities` methods to `loadEntities` for consistency. since many callers dont need paging and want the result as a list, we add `PolarisMetaStoreManager.loadEntitiesAll` as a convenient wrapper. we also remove the `PolarisEntity.nameAndId` method as callers who only need name and id should not be loading the full entity to begin with. note we rework `testCatalogNotReturnedWhenDeletedAfterListBeforeGet` from `ManagementServiceTest` because the simulated race-condition scenario can no longer happen. * review: remove filter + transformer params * review: add TODO in listPolicies * review: improve javadoc * chore: fix Page javadoc (apache#2412) Correct method args (follow-up to apache#2401) * Add 1.0.1-incubating release blog post (apache#2403) * This change fixes: (apache#2395) * use [email protected] for announcement * publish Docker images only when the vote passed (we are not suppose to publish any public artifacts before the vote is completed) * update the vote email accordingly * remove blog post link in the release announcement email to let this to the discretion of the release manager * chore(deps): update actions/setup-java action to v5 (apache#2414) * chore(test): Restore PolarisAccessManager (apache#2413) Restore `PolarisAccessManager` (with default implementation of `IcebergTokenAccessManager`) which was added as an extension point for running Polaris tests in downstream build environments under apache#789, but was mistakenly removed in apache#2343 * Update changelog prior to 1.1.0 release (apache#2406) * fix(deps): update quarkus platform and group to v3.25.4 (apache#2279) * NoSQL: merge related changes * Last merged commit 82cd416 --------- Co-authored-by: Dmitri Bourlatchkov <[email protected]> Co-authored-by: Christopher Lambert <[email protected]> Co-authored-by: JB Onofré <[email protected]> Co-authored-by: Mend Renovate <[email protected]> Co-authored-by: Pierre Laporte <[email protected]>
No description provided.