[SPARK-29062][SQL] Add V1_BATCH_WRITE to the TableCapabilityChecks #25767
Conversation
brkyvz
changed the title
|
Test build #110494 has finished for PR 25767 at commit
|
|
cc @cloud-fan @jose-torres @rdblue Can you ptal? I re-opened this after yesterday's DSV2 sync. |
|
Test build #111008 has finished for PR 25767 at commit
|
|
Test build #111010 has finished for PR 25767 at commit
|
|
Test build #111024 has finished for PR 25767 at commit
|
|
retest this please |
|
Test build #111049 has finished for PR 25767 at commit
|
| import org.apache.spark.sql.execution.datasources.v2.DataSourceV2Implicits._ | ||
| provider.getTable(dsOptions) match { | ||
| case table: SupportsWrite if table.supports(BATCH_WRITE) => | ||
| if (partitioningColumns.nonEmpty) { |
There was a problem hiding this comment.
good catch! Even if the format is a TableProvider, we may still fall back to v1. It's better to check the partition when we are really going to do a v2 write.
There was a problem hiding this comment.
BTW, technically we only need to assert no partition columns for append and overwrite. Since the v2 write here only supports append and overwrite, we can revisit it later.
|
retest this please |
|
Test build #111052 has finished for PR 25767 at commit
|
|
LGTM, merging to master! |
4 participants
What changes were proposed in this pull request?
Currently the checks in the Analyzer require that V2 Tables have BATCH_WRITE defined for all tables that have V1 Write fallbacks. This is confusing as these tables may not have the V2 writer interface implemented yet. This PR adds this table capability to these checks.
In addition, this allows V2 tables to leverage the V1 APIs for DataFrameWriter.save if they do extend the V1_BATCH_WRITE capability. This way, these tables can continue to receive partitioning information and also perform checks for the existence of tables, and support all SaveModes.
Why are the changes needed?
Partitioned saves through DataFrame.write are otherwise broken for V2 tables that support the V1
write API.
Does this PR introduce any user-facing change?
No
How was this patch tested?
V1WriteFallbackSuite