Github user nongli commented on a diff in the pull request:
https://github.com/apache/spark/pull/10498#discussion_r49003254
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/WriterContainer.scala
---
@@ -342,22 +461,34 @@ private[sql] class DynamicPartitionWriterContainer(
// If anything below fails, we should abort the task.
try {
- // This will be filled in if we have to fall back on sorting.
- var sorter: UnsafeKVExternalSorter = null
+ // If there is no sorting columns, we set sorter to null and try the
hash-based writing first,
+ // and fill the sorter if there are too many writers and we need to
fall back on sorting.
+ // If there are sorting columns, then we have to sort the data
anyway, and no need to try the
--- End diff --
I don't think it's useful to try to use the hash if it is bucketed. It's
very unlikely to bucket by < numFiles and since it's hashed, it won't be clumpy
by chance like partitions.
Feel free to address later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
