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

[SPARK-40219][SQL] Resolved view logical plan should hold the schema to avoid redundant lookup #37658

Closed
wants to merge 2 commits into from

Conversation

cloud-fan
Copy link
Contributor

@cloud-fan cloud-fan commented Aug 25, 2022

What changes were proposed in this pull request?

In CatalogImpl, we need to look up the view again to get the schema, as ResolvedView only contains the view identifier. This PR fix this issue by refactoring the ResolvedView:

  1. Split it into ResolvedPersistentView and ResolvedTempView
  2. The new logical plans hold the view schema
  3. ResolvedPersistentView holds the catalog, to match ResolvedTable.

This PR also cleans up the temp view related methods in SessionCatalog:

  1. group them together in the section Methods that interact with temp views only
  2. rename 2 methods to make the naming more consistent.

Why are the changes needed?

code cleanup and avoid redundant view lookup

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

@cloud-fan
Copy link
Contributor Author

cc @viirya @huaxingao @MaxGekk

@viirya
Copy link
Member

viirya commented Aug 25, 2022

Seems many test failures.

@cloud-fan
Copy link
Contributor Author

@viirya tests all passed now

@viirya
Copy link
Member

viirya commented Aug 30, 2022

Thanks @cloud-fan. I'll review this today.

}
tmpView
}

/**
* Resolves relations to `ResolvedTable` or `ResolvedView`. This is for resolving DDL and
Copy link
Member

Choose a reason for hiding this comment

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

ResolvedView -> ResolvedTempView and ResolvedPersistentView

@cloud-fan
Copy link
Contributor Author

thanks for review, merging to master!

@cloud-fan cloud-fan closed this in e4a73db Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants