Skip to content

API: Add SupportsReferencedBy interface #15895

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

Open
singhpk234 wants to merge 3 commits into
base: main
Choose a base branch
from
+83 −0
Open

API: Add SupportsReferencedBy interface #15895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59993b3
API: Add SupportsReferencedBy interface for referenced-by view chain …
sfc-gh-prsingh Apr 6, 2026
b6d8da2
API: Add referenced-by overloads to SessionCatalog and ViewSessionCat…
sfc-gh-prsingh Apr 6, 2026
bd9c08d
API: Improve SupportsReferencedBy Javadoc and fix capitalization
sfc-gh-prsingh Apr 13, 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
14 changes: 14 additions & 0 deletions api/src/main/java/org/apache/iceberg/catalog/SessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,20 @@ default boolean tableExists(SessionContext context, TableIdentifier ident) {
*/
Table loadTable(SessionContext context, TableIdentifier ident);

/**
* Load a table with the referenced-by view chain.
*
* @param context session context
* @param ident a table identifier
* @param referencedBy ordered list of view identifiers from outermost to innermost
* @return instance of {@link Table} implementation referred by {@code ident}
* @throws NoSuchTableException if the table does not exist
*/
default Table loadTable(
Copy link
Copy Markdown
Contributor

@stevenzwu stevenzwu Apr 7, 2026

Choose a reason for hiding this comment

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

what's the reason for adding the default here? the SupportsReferencedBy decorator interface is not extended by this interface. can we just add the proper class that implements this new interface?

Maybe only RESTSessionCatalog should implement the SupportsReferencedBy decorator interface.

Copy link
Copy Markdown
Contributor Author

@singhpk234 singhpk234 Apr 7, 2026

Choose a reason for hiding this comment

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

we have SessionContext for this api, it can't implement SupportsReferencedBy which doesn't have sessionContext

Copy link
Copy Markdown
Member

@RussellSpitzer RussellSpitzer Apr 8, 2026

Choose a reason for hiding this comment

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

Why not default to calling the parent version without "referencedBy"? I guess I have similar thoughts as Steven here.

Copy link
Copy Markdown
Member

@RussellSpitzer RussellSpitzer Apr 8, 2026

Choose a reason for hiding this comment

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

Also noted by the agents, you commented that this would delegate to old implementations for backwards compatibility :)

Copy link
Copy Markdown
Contributor Author

@singhpk234 singhpk234 Apr 8, 2026

Choose a reason for hiding this comment

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

I had that earlier version : e597a1f#diff-93c6a4d20e7c7d3a8e9b90cac139da020d6a75138c2a5718941a4c4e7b9d8217R198-R210

forgot to update the description :(, wow agents caught that, thats pretty cool !

Then i went backwards as in this scenario if we let referencedBy skip then its felt a bit incorrect as i am swallowing the intent of the api, throwing unsupported exception would better prompt the user for this is what i think, please let me know you thoughts considering above ?

SessionContext context, TableIdentifier ident, List<TableIdentifier> referencedBy) {
Copy link
Copy Markdown
Member

@RussellSpitzer RussellSpitzer Apr 8, 2026

Choose a reason for hiding this comment

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

Can referencedBy be null?

Copy link
Copy Markdown
Contributor Author

@singhpk234 singhpk234 Apr 13, 2026

Choose a reason for hiding this comment

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

it can be null, in that case it would be better for the caller to just call the loadTable(context, ident) ? i can mark this explicitly as @Nullable, wdyt ?

throw new UnsupportedOperationException("Loading a table with referenced-by is not supported");
}

/**
* Drop a table, without requesting that files are immediately deleted.
*
Expand Down
53 changes: 53 additions & 0 deletions api/src/main/java/org/apache/iceberg/catalog/SupportsReferencedBy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.iceberg.catalog;

import java.util.List;
import org.apache.iceberg.Table;
import org.apache.iceberg.view.View;

/**
* An optional catalog capability for loading tables and views with a referenced-by view chain.
*
* <p>When a table or view is loaded as part of resolving a view definition, the chain of
* referencing views can be passed to the catalog. This enables catalog servers to make
* authorization, credential-scoping, and auditing decisions based on the full view reference chain.
*
* <p>Catalogs that do not need this context are not required to implement this interface.
*/
public interface SupportsReferencedBy {
Copy link
Copy Markdown
Contributor

@stevenzwu stevenzwu Apr 7, 2026

Choose a reason for hiding this comment

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

For the universal load endpoints for relational objects, I guess we will also need to support referenced-by.
https://github.com/apache/iceberg/pull/15831/changes#diff-91e43b034980f7feb9d0d959ef4101f26254677c8ba49ad93e1edd31ecd77614

Copy link
Copy Markdown
Contributor Author

@singhpk234 singhpk234 Apr 7, 2026

Choose a reason for hiding this comment

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

referenced-by just have view identifier :), so table identifier is suff, when we define something for view specific then we would be good !

Copy link
Copy Markdown
Contributor

@stevenzwu stevenzwu Apr 8, 2026

Choose a reason for hiding this comment

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

We have been using TableIdentifier for views probably because we don't want to introduce a ViewIdentifier exactly same as TableIdentifier.

I would hope that we can align on one generic CatalogObjectIdentifier for all new REST spec and Java implementations.
https://docs.google.com/document/d/1NTQhgNbP2dkIMuXUMA5JdwliVQKCp1TU_ux5J_AaPiw/edit?tab=t.0#heading=h.lrmay9r6i8ai

We probably don't need a specific view identifier in the REST spec.

My initial comment is if we need to support referenced-by for the relations endpoint.

Copy link
Copy Markdown
Member

@RussellSpitzer RussellSpitzer Apr 8, 2026

Choose a reason for hiding this comment

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

Continuning some of the discussion on the other thread, who is this interface for? It looks like it would only be used for non-REST catalog implementations (which wouldn't use the SessionCatalog/ViewSessionCatalog)

Copy link
Copy Markdown
Contributor Author

@singhpk234 singhpk234 Apr 13, 2026

Choose a reason for hiding this comment

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

My initial comment is if we need to support referenced-by for the relations endpoint.

TableIdentifier is the universal identifier for views today — happy to adopt a generic type when it lands and yes i agree we would require to send the reference-by to the relations endpoint as well :)

who is this interface for

Its instanceof discovery pattern used by SparkCatalog as the underlying icebergCatalog that sparkCatalog holds would be implementing it like RESTCatalog [1], i plan to make RESTCatalog implement SupportsReferencedBy interface [2]
Please let me know wdyt

please ref E2E impl i have in mind : #13979


/**
* Load a table with the referenced-by view chain.
*
* @param identifier a table identifier
* @param referencedBy ordered list of view identifiers from outermost to innermost
* @return an Iceberg table
*/
Table loadTable(TableIdentifier identifier, List<TableIdentifier> referencedBy);

/**
* Load a view with the referenced-by view chain.
*
* @param identifier a view identifier
* @param referencedBy ordered list of view identifiers from outermost to innermost
* @return an Iceberg view
*/
View loadView(TableIdentifier identifier, List<TableIdentifier> referencedBy);
}
16 changes: 16 additions & 0 deletions api/src/main/java/org/apache/iceberg/catalog/ViewSessionCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ public interface ViewSessionCatalog {
*/
View loadView(SessionCatalog.SessionContext context, TableIdentifier identifier);

/**
* Load a view with the referenced-by view chain.
*
* @param context session context
* @param identifier a view identifier
* @param referencedBy ordered list of view identifiers from outermost to innermost
* @return instance of {@link View} implementation referred by the identifier
* @throws NoSuchViewException if the view does not exist
*/
default View loadView(
SessionCatalog.SessionContext context,
TableIdentifier identifier,
List<TableIdentifier> referencedBy) {
throw new UnsupportedOperationException("Loading a view with referenced-by is not supported");
}

/**
* Check whether view exists.
*
Expand Down
Loading