Github user dbtsai commented on a diff in the pull request: https://github.com/apache/spark/pull/21235#discussion_r186532502 --- Diff: sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala --- @@ -339,9 +339,16 @@ final class DataFrameWriter[T] private[sql](ds: Dataset[T]) { } } - private def assertNotBucketed(operation: String): Unit = { - if (numBuckets.isDefined || sortColumnNames.isDefined) { - throw new AnalysisException(s"'$operation' does not support bucketing right now") --- End diff -- I agree with you. I also found in `getBucketSpec`, when `numBuckets.isEmpty && sortColumnNames.isDefined`, it will throw `IllegalArgumentException`. How about alternatively, we throw `AnalysisException` for all the cases for consistency? ```scala private def getBucketSpec: Option[BucketSpec] = { assertNotSortByOrBucketedBy() numBuckets.map { n => BucketSpec(n, bucketColumnNames.get, sortColumnNames.getOrElse(Nil)) } } private def assertNotSortByOrBucketedBy(): Unit = { if (sortColumnNames.isDefined && numBuckets.isEmpty) { throw new AnalysisException("sortBy must be used together with bucketBy") } } private def assertNotBucketedAndNotSorted(operation: String): Unit = { assertNotSortByOrBucketedBy() if (numBuckets.isDefined) { if (sortColumnNames.isDefined) { throw new AnalysisException( s"'$operation' does not support bucketBy and sortBy within a bucket right now") } else { throw new AnalysisException(s"'$operation' does not support bucketBy right now") } } } ```
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org