diff --git a/docs/source/user-guide/latest/compatibility/index.md b/docs/source/user-guide/latest/compatibility/index.md index 5041856e5b..ae29a67bc1 100644 --- a/docs/source/user-guide/latest/compatibility/index.md +++ b/docs/source/user-guide/latest/compatibility/index.md @@ -121,10 +121,6 @@ divergence: raise raw Arrow errors that bypass `SparkErrorConverter` and surface as `CometNativeException` rather than `SparkArithmeticException` with the proper error class and query context ([#5072](https://github.com/apache/datafusion-comet/issues/5072)). -- `next_day` and `make_date` throw at the correct inputs but surface as `CometNativeException` - instead of `SparkIllegalArgumentException [ILLEGAL_DAY_OF_WEEK]` / - `SparkDateTimeException [DATETIME_FIELD_OUT_OF_BOUNDS.WITH_SUGGESTION]` - ([#5073](https://github.com/apache/datafusion-comet/issues/5073)). - Spark 4.2 introduced additional ANSI arithmetic overflow behavior differences that Comet does not yet track ([#4967](https://github.com/apache/datafusion-comet/issues/4967)). diff --git a/native/common/src/error.rs b/native/common/src/error.rs index baeb3a119e..81d095658e 100644 --- a/native/common/src/error.rs +++ b/native/common/src/error.rs @@ -101,6 +101,12 @@ pub enum SparkError { #[error("[DATETIME_OVERFLOW] Datetime arithmetic overflow.")] DatetimeOverflow, + #[error("[ILLEGAL_DAY_OF_WEEK] Illegal input for day of week: {input}.")] + IllegalDayOfWeek { input: String }, + + #[error("[DATETIME_FIELD_OUT_OF_BOUNDS] {range_message}. If necessary set \"spark.sql.ansi.enabled\" to \"false\" to bypass this error.")] + DatetimeFieldOutOfBounds { range_message: String }, + #[error("[INVALID_ARRAY_INDEX] The index {index_value} is out of bounds. The array has {array_size} elements. Use the SQL function get() to tolerate accessing element at invalid index and return NULL instead. If necessary set \"spark.sql.ansi.enabled\" to \"false\" to bypass this error.")] InvalidArrayIndex { index_value: i32, array_size: i32 }, @@ -276,6 +282,8 @@ impl SparkError { "IntervalArithmeticOverflowWithoutSuggestion" } SparkError::DatetimeOverflow => "DatetimeOverflow", + SparkError::IllegalDayOfWeek { .. } => "IllegalDayOfWeek", + SparkError::DatetimeFieldOutOfBounds { .. } => "DatetimeFieldOutOfBounds", SparkError::InvalidArrayIndex { .. } => "InvalidArrayIndex", SparkError::InvalidElementAtIndex { .. } => "InvalidElementAtIndex", SparkError::InvalidBitmapPosition { .. } => "InvalidBitmapPosition", @@ -458,6 +466,16 @@ impl SparkError { "suggestedFunc": suggested_func, }) } + SparkError::IllegalDayOfWeek { input } => { + serde_json::json!({ + "string": input, + }) + } + SparkError::DatetimeFieldOutOfBounds { range_message } => { + serde_json::json!({ + "rangeMessage": range_message, + }) + } SparkError::InvalidFractionOfSecond { value } => { serde_json::json!({ "value": value, @@ -605,11 +623,17 @@ impl SparkError { // DateTimeException SparkError::InvalidInputInCastToDatetime { .. } | SparkError::CannotParseTimestamp { .. } - | SparkError::InvalidFractionOfSecond { .. } => "org/apache/spark/SparkDateTimeException", + | SparkError::InvalidFractionOfSecond { .. } + | SparkError::DatetimeFieldOutOfBounds { .. } => { + "org/apache/spark/SparkDateTimeException" + } // IllegalArgumentException SparkError::DatatypeCannotOrder { .. } - | SparkError::InvalidUtf8String { .. } => "org/apache/spark/SparkIllegalArgumentException", + | SparkError::InvalidUtf8String { .. } + | SparkError::IllegalDayOfWeek { .. } => { + "org/apache/spark/SparkIllegalArgumentException" + } // FileNotFound - will be converted to SparkFileNotFoundException by the shim SparkError::FileNotFound { .. } => "org/apache/spark/SparkException", @@ -693,6 +717,8 @@ impl SparkError { // DateTime errors SparkError::CannotParseTimestamp { .. } => Some("CANNOT_PARSE_TIMESTAMP"), SparkError::InvalidFractionOfSecond { .. } => Some("INVALID_FRACTION_OF_SECOND"), + SparkError::IllegalDayOfWeek { .. } => Some("ILLEGAL_DAY_OF_WEEK"), + SparkError::DatetimeFieldOutOfBounds { .. } => Some("DATETIME_FIELD_OUT_OF_BOUNDS"), // String/UTF8 errors SparkError::InvalidUtf8String { .. } => Some("INVALID_UTF8_STRING"), diff --git a/native/spark-expr/src/datetime_funcs/make_date.rs b/native/spark-expr/src/datetime_funcs/make_date.rs index 02c9160587..b29094ba9f 100644 --- a/native/spark-expr/src/datetime_funcs/make_date.rs +++ b/native/spark-expr/src/datetime_funcs/make_date.rs @@ -25,6 +25,8 @@ use datafusion::logical_expr::{ }; use std::sync::Arc; +use crate::SparkError; + /// Spark-compatible make_date function. /// Creates a date from year, month, and day columns. /// For an invalid `(year, month, day)` triple Spark returns NULL when `spark.sql.ansi.enabled` is @@ -53,8 +55,8 @@ impl Default for SparkMakeDate { /// Build the error message Spark surfaces for an invalid date under ANSI mode. Spark wraps the /// `java.time.DateTimeException` raised by `LocalDate.of` (via `ansiDateTimeArgumentOutOfRange` / -/// `ansiDateTimeError`), so we reproduce `java.time`'s messages and validation order: month range, -/// then day range, then the day-vs-month check. +/// `ansiDateTimeError`), so we reproduce `java.time`'s messages and validation order: year range, +/// month range, day range, then the day-vs-month check. fn invalid_date_message(year: i32, month: i32, day: i32) -> String { const MONTH_NAMES: [&str; 12] = [ "JANUARY", @@ -70,6 +72,9 @@ fn invalid_date_message(year: i32, month: i32, day: i32) -> String { "NOVEMBER", "DECEMBER", ]; + if !(-999_999_999..=999_999_999).contains(&year) { + return format!("Invalid value for Year (valid values -999999999 - 999999999): {year}"); + } if !(1..=12).contains(&month) { return format!("Invalid value for MonthOfYear (valid values 1 - 12): {month}"); } @@ -178,7 +183,10 @@ impl ScalarUDFImpl for SparkMakeDate { Some(days) => builder.append_value(days), None => { if self.fail_on_error { - return Err(DataFusionError::Execution(invalid_date_message(y, m, d))); + return Err(SparkError::DatetimeFieldOutOfBounds { + range_message: invalid_date_message(y, m, d), + } + .into()); } builder.append_null(); } diff --git a/native/spark-expr/src/datetime_funcs/next_day.rs b/native/spark-expr/src/datetime_funcs/next_day.rs index df4c2f9096..fbd9defc5f 100644 --- a/native/spark-expr/src/datetime_funcs/next_day.rs +++ b/native/spark-expr/src/datetime_funcs/next_day.rs @@ -25,6 +25,8 @@ use datafusion::logical_expr::{ }; use std::sync::Arc; +use crate::SparkError; + /// Spark-compatible `next_day(start_date, day_of_week)` function. /// /// Returns the first date which is later than `start_date` and named as `day_of_week`. Unlike the @@ -146,9 +148,10 @@ impl ScalarUDFImpl for SparkNextDay { }, None => { if self.fail_on_error { - return Err(DataFusionError::Execution(format!( - "Illegal input for day of week: {day_of_week}" - ))); + return Err(SparkError::IllegalDayOfWeek { + input: day_of_week.to_string(), + } + .into()); } builder.append_null(); } diff --git a/spark/src/main/scala/org/apache/comet/serde/datetime.scala b/spark/src/main/scala/org/apache/comet/serde/datetime.scala index a74a158e97..814dd6ca6f 100644 --- a/spark/src/main/scala/org/apache/comet/serde/datetime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/datetime.scala @@ -446,12 +446,6 @@ object CometNextDay extends CometExpressionSerde[NextDay] { override def getIncompatibleReasons(): Seq[String] = DatetimeCollation.incompatibleReasons("next_day") - override def getCompatibleNotes(): Seq[String] = Seq( - "Under ANSI mode, an invalid `dayOfWeek` surfaces as `CometNativeException` rather than" + - " Spark's `SparkIllegalArgumentException` with error class `ILLEGAL_DAY_OF_WEEK`. The" + - " throw/NULL decision is correct; only the exception class and error class differ" + - " ([#5073](https://github.com/apache/datafusion-comet/issues/5073)).") - override def getSupportLevel(expr: NextDay): SupportLevel = { if (DatetimeCollation.hasNonDefaultCollation(expr)) { Incompatible(Some(collationReason)) @@ -479,11 +473,11 @@ object CometMakeDate extends CometExpressionSerde[MakeDate] { */ override def getCompatibleNotes(): Seq[String] = Seq( - "Under ANSI mode, an out-of-range `(year, month, day)` triple surfaces as" + - " `CometNativeException` rather than Spark's `SparkDateTimeException` with error class" + - " `DATETIME_FIELD_OUT_OF_BOUNDS.WITH_SUGGESTION`. The throw/NULL decision is correct;" + - " only the exception class and error class differ" + - " ([#5073](https://github.com/apache/datafusion-comet/issues/5073)).") + "Native `make_date` is limited to chrono's year range `[-262143, 262142]`; Spark accepts" + + " wider years (for example, `300000`), so Comet returns `NULL` or throws under ANSI mode" + + " for dates Spark accepts, and may incorrectly report valid dates as invalid (for example," + + " `300000-02-29` is falsely reported as not a leap year)" + + " ([#5208](https://github.com/apache/datafusion-comet/issues/5208)).") override def convert(expr: MakeDate, inputs: Seq[Attribute], binding: Boolean): Option[Expr] = { val childExpr = expr.children.map(exprToProtoInternal(_, inputs, binding)) diff --git a/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala b/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala index 09ac063cd2..6cb6448672 100644 --- a/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala +++ b/spark/src/main/spark-3.4/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala @@ -178,6 +178,16 @@ trait ShimSparkErrorConverter { Some( QueryExecutionErrors.ansiDateTimeParseError(new Exception(params("message").toString))) + case "IllegalDayOfWeek" => + Some( + QueryExecutionErrors + .ansiIllegalArgumentError(s"Illegal input for day of week: ${params("string")}")) + + case "DatetimeFieldOutOfBounds" => + Some( + QueryExecutionErrors.ansiDateTimeError( + new java.time.DateTimeException(params("rangeMessage").toString))) + case "InvalidFractionOfSecond" => Some(QueryExecutionErrors.invalidFractionOfSecondError()) diff --git a/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala b/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala index c502e4d55d..6b976e55de 100644 --- a/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala +++ b/spark/src/main/spark-3.5/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala @@ -175,6 +175,16 @@ trait ShimSparkErrorConverter { Some( QueryExecutionErrors.ansiDateTimeParseError(new Exception(params("message").toString))) + case "IllegalDayOfWeek" => + Some( + QueryExecutionErrors + .ansiIllegalArgumentError(s"Illegal input for day of week: ${params("string")}")) + + case "DatetimeFieldOutOfBounds" => + Some( + QueryExecutionErrors.ansiDateTimeError( + new java.time.DateTimeException(params("rangeMessage").toString))) + case "InvalidFractionOfSecond" => Some(QueryExecutionErrors.invalidFractionOfSecondError()) diff --git a/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala b/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala index 874a6af97c..7fb822b66b 100644 --- a/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala +++ b/spark/src/main/spark-4.x/org/apache/spark/sql/comet/shims/ShimSparkErrorConverter.scala @@ -23,8 +23,7 @@ import java.io.FileNotFoundException import scala.util.matching.Regex -import org.apache.spark.QueryContext -import org.apache.spark.SparkException +import org.apache.spark.{QueryContext, SparkException, SparkIllegalArgumentException} import org.apache.spark.sql.errors.QueryExecutionErrors import org.apache.spark.sql.execution.datasources.SchemaColumnConvertNotSupportedException import org.apache.spark.sql.types._ @@ -201,6 +200,17 @@ trait ShimSparkErrorConverter { new Exception(params("message").toString), params("suggestedFunc").toString)) + case "IllegalDayOfWeek" => + Some( + new SparkIllegalArgumentException( + errorClass = "ILLEGAL_DAY_OF_WEEK", + messageParameters = Map("string" -> params("string").toString))) + + case "DatetimeFieldOutOfBounds" => + Some( + QueryExecutionErrors.ansiDateTimeArgumentOutOfRange( + new java.time.DateTimeException(params("rangeMessage").toString))) + case "InvalidFractionOfSecond" => Some(QueryExecutionErrors.invalidFractionOfSecondError(params("value").toString.toDouble)) diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/make_date_ansi.sql b/spark/src/test/resources/sql-tests/expressions/datetime/make_date_ansi.sql index 44880e96b1..75ca530429 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/make_date_ansi.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_date_ansi.sql @@ -16,10 +16,11 @@ -- under the License. -- ANSI mode: Spark's MakeDate wraps the java.time.DateTimeException raised by LocalDate.of in --- ansiDateTimeArgumentOutOfRange (4.0, DATETIME_FIELD_OUT_OF_BOUNDS) / ansiDateTimeError (3.4/3.5) --- when spark.sql.ansi.enabled=true. Comet's native SparkMakeDate now throws the same --- java.time-style message under ANSI instead of returning NULL. The expect_error patterns below --- are substrings of that message and match across Spark versions. +-- ansiDateTimeArgumentOutOfRange (4.x) / ansiDateTimeError (3.x) when +-- spark.sql.ansi.enabled=true. The DATETIME_FIELD_OUT_OF_BOUNDS subclass and message parameters +-- vary by Spark version. Comet's native SparkMakeDate throws the same java.time-style message +-- under ANSI instead of returning NULL. The expect_error patterns below are substrings of that +-- message and match across Spark versions. -- Config: spark.sql.ansi.enabled=true -- Sentinel: a valid date must still execute natively under ANSI. This guards against the diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/next_day_ansi.sql b/spark/src/test/resources/sql-tests/expressions/datetime/next_day_ansi.sql index f289f5b34d..f7f018b4eb 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/next_day_ansi.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/next_day_ansi.sql @@ -16,7 +16,8 @@ -- under the License. -- ANSI mode: Spark's NextDay throws on a malformed dayOfWeek (SparkIllegalArgumentException / --- ILLEGAL_DAY_OF_WEEK on 3.5+, IllegalArgumentException on 3.4) when spark.sql.ansi.enabled=true. +-- ILLEGAL_DAY_OF_WEEK on 4.0+, _LEGACY_ERROR_TEMP_2000 on 3.x) when +-- spark.sql.ansi.enabled=true. -- Comet's native next_day now throws the same "Illegal input for day of week" message under ANSI -- instead of returning NULL. -- Config: spark.sql.ansi.enabled=true diff --git a/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala index 90d6992047..4df944527b 100644 --- a/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometTemporalExpressionSuite.scala @@ -21,6 +21,7 @@ package org.apache.comet import scala.util.Random +import org.apache.spark.SparkThrowable import org.apache.spark.sql.{CometTestBase, DataFrame, Row, SaveMode} import org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute import org.apache.spark.sql.catalyst.expressions.{Days, Hours, Literal} @@ -39,6 +40,47 @@ class CometTemporalExpressionSuite extends CometTestBase with AdaptiveSparkPlanH private val crossTimezones = Seq("UTC", "America/Los_Angeles", "Europe/London", "Asia/Tokyo") + private def causeChain(error: Throwable): Seq[Throwable] = + Iterator.iterate(error)(_.getCause).takeWhile(_ != null).toSeq + + private def deepestSparkThrowable(error: Throwable): SparkThrowable with Throwable = + causeChain(error) + .collect { case e: SparkThrowable with Throwable => e } + .lastOption + .getOrElse( + fail(s"No SparkThrowable in cause chain: ${causeChain(error).map(_.getClass.getName)}")) + + test("next_day and make_date ANSI errors match Spark exceptions") { + withSQLConf( + SQLConf.ANSI_ENABLED.key -> "true", + SQLConf.OPTIMIZER_EXCLUDED_RULES.key -> + "org.apache.spark.sql.catalyst.optimizer.ConstantFolding") { + Seq( + "SELECT next_day(date('2024-01-01'), 'NOT_A_DAY')", + "SELECT make_date(2024, 13, 1)", + // 999999999 instead overflows the epoch-day conversion as a plain ArithmeticException. + "SELECT make_date(1000000000, 1, 1)", + "SELECT make_date(1000000000, 13, 0)") + .foreach { query => + val df = sql(query) + checkCometOperators(stripAQEPlan(df.queryExecution.executedPlan)) + + val (sparkError, cometError) = checkSparkAnswerMaybeThrows(df) + val sparkFailure = sparkError.getOrElse(fail(s"Spark did not fail for: $query")) + val cometFailure = cometError.getOrElse(fail(s"Comet did not fail for: $query")) + val expected = deepestSparkThrowable(sparkFailure) + val actual = deepestSparkThrowable(cometFailure) + + assert(actual.getClass == expected.getClass) + assert(actual.getErrorClass == expected.getErrorClass) + assert(actual.getSqlState == expected.getSqlState) + assert(actual.getMessageParameters == expected.getMessageParameters) + assert(actual.getMessage == expected.getMessage) + assert(!causeChain(cometFailure).exists(_.isInstanceOf[CometNativeException])) + } + } + } + test("trunc (TruncDate)") { val supportedFormats = CometTruncDate.supportedFormats val unsupportedFormats = Seq("invalid")