[SPARK-32960][SQL] Provide better exception on temporary view against DataFrameWriterV2 #29830
Conversation
… DataFrameWriterV2
|
Test build #128954 has finished for PR 29830 at commit
|
| * validated to ensure it is compatible with the existing table. | ||
| * | ||
| * @throws org.apache.spark.sql.catalyst.analysis.NoSuchTableException If the table does not exist | ||
| */ |
There was a problem hiding this comment.
Should we write it about AnalysisException and update the docs above? maybe at least just for the sake of matching.
There was a problem hiding this comment.
I thought about this, and decided to skip because AnalysisException is thrown for whatever cases even without this case. We couldn't explain all these things. I'll add it if we prefer to explain it at least for explicit case.
|
cc. @cloud-fan @rdblue as well, as the issue came from #29767 (comment) |
| @throws(classOf[NoSuchTableException]) | ||
| def append(): Unit = { | ||
| assertNoTempView("append") | ||
| val append = loadTable(catalog, identifier) match { |
There was a problem hiding this comment.
I'm thinking of a more aggressive refactor, which creates an unresolved logical plan here and leaves the table/temp view lookup to the analyzer.
For example, here we can just create AppendData.byName(UnresolvedRelation(...)).
By doing this, we can make the framework more clear: the API layer should just create logical plans, other works should be done by the analyzer and query planner.
There was a problem hiding this comment.
I agree the suggestion is promising, but I'm not sure I understand all spots I need to modify. I could try to find places where InsertIntoStatement(UnresolvedRelation(...)) is resolved, but AppendData and InsertIntoStatement are not 100% same. In addition, looks like it should be done for each operation.
I'll take a look at this soon. If you can fix it easily (and you'd like to) please go ahead and I can learn from your PR.
|
Covered by #29970 |
5 participants
What changes were proposed in this pull request?
This PR adds additional check on temp view in prior to loadTable in DataFrameWriterV2, and fail-fast with better exception.
Why are the changes needed?
This PR would provide better exception and its message - see below section.
Does this PR introduce any user-facing change?
Yes - previously using temp view in DataFrameWriterV2 throws NoSuchTableException without any information on temp view. Now it throws AnalysisException with exception message that it's a temporary view which doesn't support write operations in DataFrameWriterV2.
How was this patch tested?
New UTs