You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/09/22 12:31:53 UTC
[GitHub] [spark] wang-zhun opened a new pull request #34072: [SPARK-36680][CATALYST] Supports Dynamic Table Options for Spark SQL
wang-zhun opened a new pull request #34072:
URL: https://github.com/apache/spark/pull/34072
### What changes were proposed in this pull request?
Add a new hint `OPTION`
### Why are the changes needed?
Now a DataFrame API user can implement dynamic options through the DataFrameReader$option method, but Spark SQL users cannot use.
```
public interface SupportsRead extends Table {
ScanBuilder newScanBuilder(CaseInsensitiveStringMap var1);
}
```
The table options were persisted to the Catalog and if we want to modify that, we should use another DDL like "ALTER TABLE ...". But there are some cases that user want to modify the table options dynamically just in the query:
- JDBCTable set fetchsize according to the actual situation of the table
- IcebergTable support time travel
```
spark.read
.option("snapshot-id", 10963874102873L)
.format("iceberg")
.load("path/to/table")
```
These parameters setting is very common and ad-hoc, setting them flexibly would promote the user experience with Spark SQL especially for Now we support catalog expansion.
### Does this PR introduce _any_ user-facing change?
##### Partitioning Hints
```
-- time trave
SELECT * FROM t /*+ OPTIONS('snapshot-id'='10963874102873L') */
```
### How was this patch tested?
Added Unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wmoustafa commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wmoustafa commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-969847199
> BTW, what's the benefit over having:
>
> ```sql
> -- option `key` being passed with `value`
> SET spark.datasource.jdbc.key=value;
> SELECT * FROM jdbc_catalog.db.table;
> RESET spark.datasource.jdbc.key;
> ```
>
> besides that it's just shorter?
Those are table-level options so one may set different key/value pairs per table within the same query. Also the setting and resetting approach is not friendly to concurrent queries.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiasheng55 commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
jiasheng55 commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-964034783
Hi, @rdblue , this dynamic table option feature is really helpful when processing Iceberg tables, could you help to take a look at this PR, thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #34072: [SPARK-36680][CATALYST] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34072:
URL: https://github.com/apache/spark/pull/34072#discussion_r714462840
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1244,15 +1245,21 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
* }}}
*/
override def visitTable(ctx: TableContext): LogicalPlan = withOrigin(ctx) {
- UnresolvedRelation(visitMultipartIdentifier(ctx.multipartIdentifier))
+ val tableId = visitMultipartIdentifier(ctx.multipartIdentifier)
+ val options = Option(ctx.optionHint).map(hint =>
+ visitPropertyKeyValues(hint.options)).getOrElse(Map.empty)
+ UnresolvedRelation(tableId, new CaseInsensitiveStringMap(options.asJava))
}
/**
* Create an aliased table reference. This is typically used in FROM clauses.
*/
override def visitTableName(ctx: TableNameContext): LogicalPlan = withOrigin(ctx) {
val tableId = visitMultipartIdentifier(ctx.multipartIdentifier)
- val table = mayApplyAliasPlan(ctx.tableAlias, UnresolvedRelation(tableId))
+ val options = Option(ctx.optionHint).map(hint =>
+ visitPropertyKeyValues(hint.options)).getOrElse(Map.empty)
+ val table = mayApplyAliasPlan(ctx.tableAlias,
+ UnresolvedRelation(tableId, new CaseInsensitiveStringMap(options.asJava)))
Review comment:
I don't think this works for the tables already defined with options because Spark respects the table properties defined in the table. This `options` is only for DSv2 for now. Can you add an e2e test, and see if it works? e.g.) create a table view and set the option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #34072: [SPARK-36680][CATALYST] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34072:
URL: https://github.com/apache/spark/pull/34072#discussion_r714462840
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -1244,15 +1245,21 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with SQLConfHelper with Logg
* }}}
*/
override def visitTable(ctx: TableContext): LogicalPlan = withOrigin(ctx) {
- UnresolvedRelation(visitMultipartIdentifier(ctx.multipartIdentifier))
+ val tableId = visitMultipartIdentifier(ctx.multipartIdentifier)
+ val options = Option(ctx.optionHint).map(hint =>
+ visitPropertyKeyValues(hint.options)).getOrElse(Map.empty)
+ UnresolvedRelation(tableId, new CaseInsensitiveStringMap(options.asJava))
}
/**
* Create an aliased table reference. This is typically used in FROM clauses.
*/
override def visitTableName(ctx: TableNameContext): LogicalPlan = withOrigin(ctx) {
val tableId = visitMultipartIdentifier(ctx.multipartIdentifier)
- val table = mayApplyAliasPlan(ctx.tableAlias, UnresolvedRelation(tableId))
+ val options = Option(ctx.optionHint).map(hint =>
+ visitPropertyKeyValues(hint.options)).getOrElse(Map.empty)
+ val table = mayApplyAliasPlan(ctx.tableAlias,
+ UnresolvedRelation(tableId, new CaseInsensitiveStringMap(options.asJava)))
Review comment:
I don't think this works for the tables already defined with options because Spark respects the table properties defined in the table. This `options` is only for DSv2 for now. Can you add an e2e test, and see if it works? e.g.) create a table view and set the option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wang-zhun commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wang-zhun commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-983318289
Close this PR first and look forward to a better proposal implementation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #34072: [SPARK-36680][CATALYST] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34072:
URL: https://github.com/apache/spark/pull/34072#discussion_r714476788
##########
File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/PlanParserSuite.scala
##########
@@ -1084,6 +1087,18 @@ class PlanParserSuite extends AnalysisTest {
table("testcat", "db", "tab").select(star()).hint("BROADCAST", $"tab"))
}
+ test("option hint") {
Review comment:
let's add a JIRA prefix
```suggestion
test("SPARK-36680: option hint") {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wang-zhun commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wang-zhun commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-968464115
> It seems weird to have it as a hint in SQL select statement (not clear that it's part of a table scan). Maybe better as a TVF argument?
@rxin Thank your for your suggestion. It is very useful for us, we did not notice TVF before
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-969849020
Can we fix the PR description better for that point in PR description? There seems already a way to resolve the issue PR description explains.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-965909731
Shall we initiate a discussion on Spark dev mailing list? I would like to have this feature too but not sure if the syntax makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wang-zhun closed pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wang-zhun closed pull request #34072:
URL: https://github.com/apache/spark/pull/34072
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] coolderli commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
coolderli commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-925639075
@RussellSpitzer @aokolnychyi Could you please take a look, Thanks?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-965909731
Shall we initiate a discussion on Spark dev mailing list? I would like to have this feature either but not sure if the syntax makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen edited a comment on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
xkrogen edited a comment on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-964356340
Haven't had a chance to look through the implementation yet, but big +1 on the feature from my side. We maintain an internal DSv2 source that requires options to leverage more advanced functionality, and currently it's not possible to use those features via the pure SQL API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
xkrogen commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-964356340
Haven't had a chance to look through the PR yet, but big +1 on the feature from my side. We maintain an internal DSv2 source that requires options to leverage more advanced functionality, and currently it's not possible to use those features via the pure SQL API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wmoustafa commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wmoustafa commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-964398993
+1 on the feature as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wmoustafa commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wmoustafa commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-970642459
There is also a catch for allowing the hints framework to control the physical plan properties as opposed to query semantics, which is that it does not seem straightforward to enforce that (i.e., preventing time travel from being communicated through hints). Does it make sense to make the `OPTIONS` API a top level SQL keyword outside the hints framework? This also aligns with the style in the counterpart Scala API (which does not leverage hints). Further, we would not have to worry about whether the option defines physical vs semantic behavior (which is also the case in the Scala version).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-969844990
BTW, what's the benefit over having:
```sql
-- option `key` being passed with `value`
SET spark.datasource.jdbc.key=value;
SELECT * FROM jdbc_catalog.db.table;
RESET spark.datasource.jdbc.key;
```
besides that it's just shorter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-926271534
yeah, I agree that the idea makes sense too. one thing is if the syntax makes sense ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wmoustafa commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wmoustafa commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-969262647
It seems that there are two types of options that can be supported through this API: options that do not change the query semantics/results and only affect physical plan choices and options that affect the query semantics. Examples of the former include setting the split size, while examples of the latter include setting snapshot ID or the timestamp of time travel queries. I think the latter mostly applies to temporal/versioning queries. Using the hints API sounds acceptable for the former. [Hints + Options API is used in Flink]( https://nightlies.apache.org/flink/flink-docs-master/docs/dev/table/sql/queries/hints/), but seems to control physical plan options still. If most of the semantic options (i.e., beyond physical) revolve around snapshots, versioning, and time travel, we can consider the SQL 2011 Standard which [added support for temporal databases](https://en.wikipedia.org/wiki/SQL:2011#Temporal_support).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wang-zhun commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wang-zhun commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-963889909
@HyukjinKwon Help review when you have time, thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xkrogen edited a comment on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
xkrogen edited a comment on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-964356340
Haven't had a chance to look through the implementation yet, but big +1 on the feature from my side. We maintain an internal DSv2 source that requires options to leverage more advanced functionality, and currently it's not possible to use those features via the pure SQL API.
cc @wmoustafa
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wang-zhun commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
wang-zhun commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-926576941
Thanks @HyukjinKwon @RussellSpitzer for your particular review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rdblue commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-964343090
Overall, this change makes sense to me. This isn't a great way to time travel because we want to load the table at a specific version/snapshot or time. But @huaxingao is working on that in a separate issue. For everything else, this is a great improvement. Thank you, @wang-zhun!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rxin commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
rxin commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-968451305
It seems weird to have it as a hint in SQL select statement (not clear that it's part of a table scan). Maybe better as a TVF argument?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-969698483
Just an FYI： I am working on time travel in this PR https://github.com/apache/spark/pull/34497
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34072: [SPARK-36680][CATALYST] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-924887889
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] RussellSpitzer commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-926001624
This seems like a reasonable change to me, would we want to do similar things for write? Or would this also work for write if the relation is being written to rather than being read from?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] coolderli commented on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
coolderli commented on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-925637031
This feature is very useful. We can dynamic adjustment table options when we submit a query. For example, we can implement time travel by spark SQL on the iceberg.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon edited a comment on pull request #34072: [SPARK-36680][SQL] Supports Dynamic Table Options for Spark SQL
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #34072:
URL: https://github.com/apache/spark/pull/34072#issuecomment-969849020
Can we fix the PR description better for that point? There seems already a way to resolve the issue PR description explains.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org