peter-toth commented on code in PR #57508:
URL: https://github.com/apache/spark/pull/57508#discussion_r3650533789
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala:
##########
@@ -358,6 +364,12 @@ class RelationResolution(
CatalogV2Util.lookupCachedRelation(sharedRelationCache, catalog, ident,
table, conf)
}
+ private def applyOptions(
+ cached: LogicalPlan,
+ options: CaseInsensitiveStringMap): LogicalPlan = cached transform {
+ case r: DataSourceV2Relation => r.copy(options = options)
Review Comment:
**Finding 2.** This only rewrites `DataSourceV2Relation`, so the same bug
survives for v1 relations. A v1 table (session-catalog data-source or Hive
table) resolves through `createDataSourceV1Scan` ->
`SessionCatalog.getRelation(v1Table, options)` -> `SubqueryAlias(_,
UnresolvedCatalogRelation(metadata, options))` (`SessionCatalog.scala:1140`),
that plan goes into the same `relationCache`, and the options on it are live:
`FindDataSourceTable` hands them to `readDataSourceTable` ->
`DataSourceUtils.generateDatasourceOptions` (`DataSourceStrategy.scala:382`,
`:259`), which even rebuilds a cached `HadoopFsRelation` when the options
differ. v1 dynamic options aren't theoretical - `DDLSuite.scala:1394`
("SPARK-51747: Data source cached plan respects options if ignore conf
disabled") asserts that `SELECT * FROM t WITH ('delimiter' = ';')` on a CSV
table changes the returned rows. So `INSERT INTO v1t WITH (a) SELECT * FROM v1t
WITH (b)`, or a plain self-join on a v1 table, still ends up
with one set of options for both references.
`UnresolvedCatalogRelation` is already imported here, so it's a one-liner:
```suggestion
case r: DataSourceV2Relation => r.copy(options = options)
case r: UnresolvedCatalogRelation => r.copy(options = options)
```
If you'd rather keep this PR DSv2-only, could you say that in the
description and leave a short note here? This PR removes the `AstBuilder`
comment that was the only record of the limitation.
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2OptionSuite.scala:
##########
@@ -121,6 +121,44 @@ class DataSourceV2OptionSuite extends DatasourceV2SQLBase {
}
}
+ test("SPARK-58330: dynamic options are not lost when INSERT selects from the
same table") {
Review Comment:
**Finding 3.** All three new tests pair a write reference with a read
reference. The plainest instance of the bug - two *read* references in one
query - isn't covered, and that's the case where the fix changes behavior in
both directions (neither scan inherits the other's options). On master both
scans come out with `split-size = 5`, because the second reference hits the
cache. Roughly ten lines here:
```scala
test("SPARK-58330: each reference in a self-join keeps its own dynamic
options") {
val t1 = s"${catalogAndNamespace}table"
withTable(t1) {
sql(s"CREATE TABLE $t1 (id bigint, data string)")
sql(s"INSERT INTO $t1 VALUES (1, 'a'), (2, 'b')")
val df = sql(s"SELECT a.id FROM $t1 WITH (`split-size` = 5) a " +
s"JOIN $t1 WITH (`split-size` = 9) b ON a.id = b.id")
val splitSizes = df.queryExecution.optimizedPlan.collect {
case s: DataSourceV2ScanRelation =>
s.relation.options.get("split-size")
}
assert(splitSizes.sorted === Seq("5", "9"))
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
