rdblue commented on a change in pull request #25115: [SPARK-28351][SQL] Support
DELETE in DataSource V2
URL: https://github.com/apache/spark/pull/25115#discussion_r313512216
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceResolution.scala
##########
@@ -173,6 +173,19 @@ case class DataSourceResolution(
// only top-level adds are supported using AlterTableAddColumnsCommand
AlterTableAddColumnsCommand(table, newColumns.map(convertToStructField))
+ case DeleteFromStatement(AsTableIdentifier(table), tableAlias, condition)
=>
+ throw new AnalysisException(
Review comment:
I don't think we need to update `ResolveTables`, though I do see that it
would be nice to use `ResolveTables` as the only rule that resolves
`UnresolvedRelation` for v2 tables.
There is already another rule that loads tables from a catalog,
[`ResolveInsertInto`](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L772).
I considered updating that rule and moving the table resolution part into
`ResolveTables` as well, but I think it is a little cleaner to resolve the
table when converting the statement (in `DataSourceResolution`), as @cloud-fan
is suggesting.
One of the reasons to do this for the insert plans is that those plans don't
include the target relation as a child. Instead, those plans have the data to
insert as a child node, which means that the unresolved relation won't be
visible to the `ResolveTables` rule.
Taking the same approach in this PR would also make this a little cleaner.
If `DeleteFrom` didn't expose the relation as a child, it could be a
`UnaryNode` and you wouldn't need to update some of the other rules to
explicitly include `DeleteFrom`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
