Skip to content

deduplicate storage requests when loading views. #3488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dimas-b merged 9 commits into from
Jan 23, 2026
Merged

deduplicate storage requests when loading views. #3488

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7d097a1
create file io out side of lambdas
evindj Jan 18, 2026
b3648e2
dedup metadata fetch by reusing ops
evindj Jan 20, 2026
8aa8074
Added unit tests
evindj Jan 21, 2026
a3bb73a
spotless apply
evindj Jan 21, 2026
68e16a0
review comments
evindj Jan 22, 2026
eeaae08
keep inefficient retry for a different PR
evindj Jan 22, 2026
00a5d91
move comment location
evindj Jan 22, 2026
ac443da
cleanup
evindj Jan 22, 2026
3b7ceda
cleanup
evindj Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@
import org.apache.iceberg.util.PropertyUtil;
import org.apache.iceberg.util.Tasks;
import org.apache.iceberg.view.BaseMetastoreViewCatalog;
import org.apache.iceberg.view.BaseView;
import org.apache.iceberg.view.View;
import org.apache.iceberg.view.ViewBuilder;
import org.apache.iceberg.view.ViewMetadata;
import org.apache.iceberg.view.ViewMetadataParser;
Expand Down Expand Up @@ -810,6 +812,24 @@ protected ViewOperations newViewOps(TableIdentifier identifier) {
return new BasePolarisViewOperations(catalogFileIO, identifier);
}

/**
* Override to fix a bug in {@link BaseMetastoreViewCatalog#loadView} where {@link #newViewOps} is
Copy link
Contributor

@dimas-b dimas-b Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tangential: Could you also double check for something like this in tables?

Copy link
Contributor

@dimas-b dimas-b Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evindj : any insight here? it's not required for current PR, but since you're involved in this issue already, I thought it might be easy for you to quickly double check that 😉

Copy link
Contributor Author

@evindj evindj Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is my understanding that this would not happen for LoadTable, because loadtable directly uses the operations from the table to construct the response. See https://github.com/apache/polaris/blob/main/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java#L387

* called twice, causing redundant metadata fetches. This implementation reuses the same {@link
* ViewOperations} instance.
*/
@Override
public View loadView(TableIdentifier identifier) {
if (isValidIdentifier(identifier)) {
ViewOperations ops = newViewOps(identifier);
if (ops.current() == null) {
throw new NoSuchViewException("View does not exist: %s", identifier);
}
return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
}

throw new NoSuchViewException("Invalid view identifier: %s", identifier);
}

@Override
public boolean dropView(TableIdentifier identifier) {
boolean purge =
Expand Down