-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-28998][SQL] reorganize the packages of DS v2 interfaces/classes #25700
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
Conversation
|
Test build #110192 has finished for PR 25700 at commit
|
|
most of the changes are import change. |
|
Test build #110193 has finished for PR 25700 at commit
|
|
Test build #110350 has finished for PR 25700 at commit
|
| * ) | ||
| * </pre> | ||
| */ | ||
| @Experimental |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with this, but we should be more careful to make sure these annotations are included earlier in future PRs.
|
+1 The renames look good to me. As long as there are no added changes, then rebasing and committing this when tests pass is okay with me. |
| * A mix-in interface for {@link Table} delete support. Data sources can implement this | ||
| * interface to provide the ability to delete data from tables that matches filter expressions. | ||
| */ | ||
| @Experimental |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we also need to add the since tags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently none of the DS v2 interfaces have the since tag. We can add them all together later.
| * A thread-safe manager for [[CatalogPlugin]]s. It tracks all the registered catalogs, and allow | ||
| * the caller to look up a catalog by name. | ||
| */ | ||
| private[sql] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we not want to make this API public, similar to the catalog implementation we had before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, CatalogManager is a completely internal stuff.
brkyvz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Hoping this is the last ever package change for DSV2...
|
Test build #110483 has finished for PR 25700 at commit
|
|
Test build #110507 has finished for PR 25700 at commit
|
|
thanks for the review, merging to master! |
### What changes were proposed in this pull request? reorganize the packages of DS v2 interfaces/classes: 1. `org.spark.sql.connector.catalog`: put `TableCatalog`, `Table` and other related interfaces/classes 2. `org.spark.sql.connector.expression`: put `Expression`, `Transform` and other related interfaces/classes 3. `org.spark.sql.connector.read`: put `ScanBuilder`, `Scan` and other related interfaces/classes 4. `org.spark.sql.connector.write`: put `WriteBuilder`, `BatchWrite` and other related interfaces/classes ### Why are the changes needed? Data Source V2 has evolved a lot. It's a bit weird that `Expression` is in `org.spark.sql.catalog.v2` and `Table` is in `org.spark.sql.sources.v2`. ### Does this PR introduce any user-facing change? No ### How was this patch tested? existing tests Closes apache#25700 from cloud-fan/package. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
What changes were proposed in this pull request?
reorganize the packages of DS v2 interfaces/classes:
org.spark.sql.connector.catalog: putTableCatalog,Tableand other related interfaces/classesorg.spark.sql.connector.expression: putExpression,Transformand other related interfaces/classesorg.spark.sql.connector.read: putScanBuilder,Scanand other related interfaces/classesorg.spark.sql.connector.write: putWriteBuilder,BatchWriteand other related interfaces/classesWhy are the changes needed?
Data Source V2 has evolved a lot. It's a bit weird that
Expressionis inorg.spark.sql.catalog.v2andTableis inorg.spark.sql.sources.v2.Does this PR introduce any user-facing change?
No
How was this patch tested?
existing tests