[SPARK-39799][SQL] DataSourceV2: View catalog interface by jzhuge · Pull Request #37556 · apache/spark

jzhuge · 2022-08-17T16:25:33Z

What changes were proposed in this pull request?

ViewCatalog API described in SPIP.

Why are the changes needed?

First step towards DataSourceV2 view support.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

N/A

holdenk · 2022-08-18T18:02:50Z

cc @dongjoon-hyun who had comments on the SPIP

dongjoon-hyun · 2022-08-18T18:10:52Z

Thank you for pinging me, @holdenk .

cc @viirya , @sunchao , @huaxingao , @aokolnychyi , @RussellSpitzer

holdenk

This looks a good first PR for the view catalog interface to me, I think it would be useful to add a link to a working implementation of this (I think having API and impl come in seperately is fine but good to have a known impl of the API existing to know that we've got a working version of it present).

jzhuge · 2022-08-18T19:59:54Z

Find this message strange from https://github.com/jzhuge/spark/runs/7886846067?check_suite_focus=true:

[info] Found the following changed modules: catalyst, hive-thriftserver

This PR does not change module hive-thriftserver.

holdenk · 2022-08-22T22:29:16Z

Interesting, the failure in CliSuite seems like a red-herring (e.g. that text is present), maybe try a re-base + re-run?

holdenk · 2022-11-03T02:06:01Z

pending CI and any other concerns from others I plan to merge this on Friday.

jzhuge · 2022-11-03T03:19:27Z

Looking at error:

SparkThrowableSuite.Error classes are correctly formatted

jzhuge · 2022-11-05T19:02:30Z

Puzzled by this pyspark test failures. Seems unrelated.

[info] compiling 25 Scala sources to /__w/spark/spark/connector/docker-integration-tests/target/scala-2.12/test-classes ...
[error] /__w/spark/spark/sql/hive/src/test/scala/org/apache/spark/sql/HiveCharVarcharTestSuite.scala:49:13: exception during macro expansion: 
[error] java.util.MissingResourceException: Can't find bundle for base name org.scalactic.ScalacticBundle, locale en_US

xkrogen · 2022-11-07T18:17:21Z

Current diff LGTM. We've been testing this out internally (including changes from the umbrella PR as well) and with the recent changes I think this PR looks like it should set us up for success with the rest of the feature.

jzhuge · 2022-11-07T18:43:25Z

Looking at error:

SparkThrowableSuite.Error classes are correctly formatted

Fixed

ljfgem · 2022-11-10T20:23:18Z

Hi @jzhuge @holdenk, can we merge this PR? Or is there any other concerns?

holdenk · 2022-11-16T16:59:52Z

LGTM I'll merge this now to the current dev branch.

jzhuge · 2022-11-16T17:36:38Z

Thanks @holdenk, @wmoustafa, @xkrogen, @ljfgem for the reviews!

cloud-fan · 2022-11-21T16:04:00Z

+  String[] columnComments();
+
+  /**
+   * The view properties.


I don't have a strong opinion, but shall we put everything (except for the query and schema) into view properties with reserved property keys? TableCatalog was designed this way.

Thanks for the feedback!

The goal is to capture the entire CREATE VIEW syntax in API. Leave the choice to catalog plugin developers on whether to use properties to implement the APIs. I'd expect HMS-backed view catalog implementation will continue to do so the same as the current v1 implementation. On the other hand, view catalog plugins that support more free-form storage formats such as JSON can choose a different approach.

### What changes were proposed in this pull request? ViewCatalog API described in [SPIP](https://docs.google.com/document/d/1XOxFtloiMuW24iqJ-zJnDzHl2KMxipTjJoxleJFz66A/edit?usp=sharing). ### Why are the changes needed? First step towards DataSourceV2 view support. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? N/A Closes apache#37556 from jzhuge/SPARK-39799. Authored-by: John Zhuge <jzhuge@apache.org> Signed-off-by: Holden Karau <hkarau@netflix.com>

…iewCatalog ### What changes were proposed in this pull request? ViewCatalog API improvements described in [SPIP](https://docs.google.com/document/d/1XOxFtloiMuW24iqJ-zJnDzHl2KMxipTjJoxleJFz66A/edit?usp=sharing) that didn't make it into the codebase as part of #37556 ### Why are the changes needed? Required for DataSourceV2 view support. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? N/A ### Was this patch authored or co-authored using generative AI tooling? N/A Closes #43677 from nastra/SPARK-45807. Authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com> Signed-off-by: Holden Karau <holden@pigscanfly.ca>

…iewCatalog ### What changes were proposed in this pull request? ViewCatalog API improvements described in [SPIP](https://docs.google.com/document/d/1XOxFtloiMuW24iqJ-zJnDzHl2KMxipTjJoxleJFz66A/edit?usp=sharing) that didn't make it into the codebase as part of apache#37556 ### Why are the changes needed? Required for DataSourceV2 view support. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? N/A ### Was this patch authored or co-authored using generative AI tooling? N/A Closes apache#43677 from nastra/SPARK-45807. Authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com> Signed-off-by: Holden Karau <holden@pigscanfly.ca>

github-actions Bot added the SQL label Aug 17, 2022

holdenk reviewed Aug 18, 2022

View reviewed changes

Comment thread sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/View.java Outdated

jzhuge force-pushed the SPARK-39799 branch from ca3ea2e to 28725e8 Compare August 22, 2022 23:04

jzhuge force-pushed the SPARK-39799 branch from 28725e8 to 07ad47c Compare August 30, 2022 19:18

jzhuge force-pushed the SPARK-39799 branch 2 times, most recently from 766b6dd to f675201 Compare September 17, 2022 01:39

jzhuge force-pushed the SPARK-39799 branch from f675201 to b332f76 Compare November 3, 2022 00:10

github-actions Bot added the CORE label Nov 3, 2022

jzhuge force-pushed the SPARK-39799 branch from b332f76 to 95662a6 Compare November 3, 2022 03:43

wmoustafa reviewed Nov 4, 2022

View reviewed changes

Comment thread sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/View.java Outdated

jzhuge force-pushed the SPARK-39799 branch from 95662a6 to 2b1346b Compare November 4, 2022 23:28

xkrogen reviewed Nov 4, 2022

View reviewed changes

Comment thread sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewCatalog.java Outdated

jzhuge force-pushed the SPARK-39799 branch 2 times, most recently from d325925 to cd0c2a2 Compare November 4, 2022 23:38

ljfgem reviewed Nov 5, 2022

View reviewed changes

Comment thread sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/ViewCatalog.java Outdated

[SPARK-39799][SQL] DataSourceV2: View catalog interface

ea1cb90

jzhuge force-pushed the SPARK-39799 branch from cd0c2a2 to ea1cb90 Compare November 5, 2022 19:41

jzhuge mentioned this pull request Nov 7, 2022

API: Add view interfaces apache/iceberg#4925

Merged

asfgit closed this in d12abff Nov 16, 2022

cloud-fan reviewed Nov 21, 2022

View reviewed changes

nastra mentioned this pull request Nov 6, 2023

[SPARK-45807][SQL] Add createOrReplaceView(..) / replaceView(..) to ViewCatalog #43677

Closed

Uh oh!

Conversation

jzhuge commented Aug 17, 2022

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

holdenk commented Aug 18, 2022

Uh oh!

dongjoon-hyun commented Aug 18, 2022

Uh oh!

holdenk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jzhuge commented Aug 18, 2022

Uh oh!

holdenk commented Aug 22, 2022

Uh oh!

holdenk commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jzhuge commented Nov 3, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jzhuge commented Nov 5, 2022

Uh oh!

xkrogen commented Nov 7, 2022

Uh oh!

jzhuge commented Nov 7, 2022

Uh oh!

ljfgem commented Nov 10, 2022

Uh oh!

holdenk commented Nov 16, 2022

Uh oh!

jzhuge commented Nov 16, 2022

Uh oh!

cloud-fan Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jzhuge Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

holdenk commented Nov 3, 2022 •

edited

Loading

cloud-fan Nov 21, 2022 •

edited

Loading