Add support for timeParserPolicy=LEGACY #2875
Conversation
docs/compatibility.md
Outdated
| @@ -353,7 +353,12 @@ the specified format string will fall into one of three categories: | |||
| - Supported on GPU but may produce different results to Spark | |||
| - Unsupported on GPU | |||
|
|
|||
| The formats which are supported on GPU and 100% compatible with Spark are : | |||
| The formats which are supported on GPU and 100% compatible with Spark vary depending on the setting | |||
There was a problem hiding this comment.
But from reading below none of the formats are 100% compatible with Spark because even for CORRECTED and EXCEPTION we do not detect trailing characters.
There was a problem hiding this comment.
This was badly worded. We do claim that some of the formats are 100% compatible. I have made some changes to try and make this clearer.
| if (!conf.incompatDateFormats) { | ||
| willNotWorkOnGpu(s"LEGACY format '$sparkFormat' on the GPU is not guaranteed " + | ||
| s"to produce the same results as Spark on CPU. Set " + | ||
| s"spark.rapids.sql.incompatibleDateFormats.enabled=true to force onto GPU.") |
There was a problem hiding this comment.
nit. Instead of hard coding the config here can we use the KEY form the RapidsConf just to avoid any possible misspellings?
andygrove
changed the title
|
During manual performance testing, I ran into some behavior that I don't understand yet, so I am changing this to a draft / WIP for now. |
andygrove
changed the title
The issue was that I had ANSI mode enabled when manually testing, and hadn't implemented ANSI support as part of this PR. It is now updated to fall back to CPU if LEGACY + ANSI are both enabled. If we do want to support LEGACY + ANSI together then we can do that as a follow-on issue. |
Signed-off-by: Andy Grove <andygrove@nvidia.com>
andygrove
force-pushed
the
legacy-timeParserPolicy
branch
from
93d6b76 to
3b6bc16
Compare
…und during manual fuzzing Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
|
build |
Signed-off-by: Andy Grove <andygrove@nvidia.com>
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
Signed-off-by: Andy Grove <andygrove@nvidia.com>
| // We are compatible with Spark for these formats when the timeParserPolicy is LEGACY. It | ||
| // is possible that other formats may be supported but these are the only ones that we have | ||
| // tests for. | ||
| val LEGACY_COMPATIBLE_FORMATS = Seq( |
There was a problem hiding this comment.
since we are doing lookups in this Seq can we make it a Set[LegacyParseFormat] or even better a Map[String, LegacyParseFormat]
Signed-off-by: Andy Grove <andygrove@nvidia.com>
|
build |
Closes #2860
This PR adds support for parsing strings to date/timestamp when
spark.sql.legacy.timeParserPolicy=LEGACYfor the following formats:dd-MM-yyyydd/MM/yyyyyyyy/MM/ddyyyy-MM-ddyyyy/MM/dd HH:mm:ssyyyy-MM-dd HH:mm:ssWe are not 100% compatible with Spark on CPU in all cases so this support is only enabled when
spark.rapids.sql.incompatibleDateFormats.enabledis also set to true. We have the following limitations when running on the GPU: