Skip to content
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.

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

Core: Fix REST catalog handling when the service has no view support #9853

Merged

Conversation

rdblue
Copy link
Contributor

@rdblue rdblue commented Mar 2, 2024

This is a fix for Spark table loading when a REST catalog does not support view extensions.

The problem is that Spark runs ResolveViews before ResolveTables in the substitution batch of rules. With view support in the REST catalog, table references will result in a loadView call to check if the unresolved identifier is a view. If the catalog service does not support views, then a 4XX exception is thrown but not caught and used to indicate the view does not exist.

This translates 4XX calls returned by the load view path to NoSuchViewException indicating that the service does not support views.

@github-actions github-actions bot added the core label Mar 2, 2024
rdblue added 2 commits March 2, 2024 11:05

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@rdblue
Copy link
Contributor Author

rdblue commented Mar 2, 2024

Okay, I can confirm that this fixes the problem locally.

@danielcweeks danielcweeks merged commit 1a4f23b into apache:main Mar 2, 2024
41 checks passed
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar left a comment

Choose a reason for hiding this comment

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

I'm just a bit late to this, but I also verified the behavior before/after this change in Spark. This change looks good to me.

Copy link
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@ajantha-bhat
Copy link
Member

ajantha-bhat commented Mar 3, 2024

LGTM.

How can we catch these errors early?
How can we add the testcase? via CatalogHnadlers or do we need a REST catalog reference implementation for this?

devangjhabakh pushed a commit to cdouglas/iceberg that referenced this pull request Apr 22, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…pache#9853)

* Core: Fix REST catalog handling when the service has no view support.

* Fix error message.

* Improve the error message.
@nastra nastra mentioned this pull request Aug 14, 2024
6 tasks
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…pache#9853)

* Core: Fix REST catalog handling when the service has no view support.

* Fix error message.

* Improve the error message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants