[FEA] Support yyyyMMdd under CORRECTED policy [databricks] - #15820
[FEA] Support yyyyMMdd under CORRECTED policy [databricks]#15820gerashegalov wants to merge 4 commits into
Conversation
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
|
build |
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
|
build |
Greptile SummaryThe PR enables GPU handling of packed
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains in the reviewed changes. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[yyyyMMdd string expression] --> B{Parser policy}
B -->|CORRECTED| C{Extended years enabled?}
C -->|No| D[GPU fused parser]
C -->|Yes| E[CPU fallback]
B -->|EXCEPTION| E
D --> F[Spark-compatible result or ANSI error]
E --> F
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
| "MM-yyyy", | ||
| "MM/dd/yyyy", | ||
| "MM-dd-yyyy", | ||
| "yyyyMMdd", |
There was a problem hiding this comment.
This list claims to be for both CORRECTED and EXCEPTION, and then there is a separate list for LEGACY, but I think that is misleading. EXCEPTION is supposed to:
- try the CORRECTED parser (returning on success)
- if it fails try the LEGACY parser
- if the LEGACY parser succeeds throw SparkUpgradeException
Per step 2 that means being supported under EXCEPTION also requires LEGACY to produce valid success/failure semantics. E.g. AI came up with the combination of 2024101 to yyyyMMdd under EXCEPTION, which should fail (Spark LEGACY accepts it) but we succeed (the JNI throws on it in LEGACY). I think we need three compatibility lists: CORRECTED, EXCEPTION, and LEGACY, where atm yyyyMMdd is not EXCEPTION compatible.
There was a problem hiding this comment.
Good catch. Fixed in 18bd197 by separating EXCEPTION_COMPATIBLE_FORMATS from CORRECTED_COMPATIBLE_FORMATS and selecting the policy-specific set during both tagging and execution. yyyyMMdd now falls back to CPU under EXCEPTION, so Spark preserves the CORRECTED/LEGACY disagreement behavior. I added coverage that asserts GetTimestamp fallback for a normal yyyyMMdd value and the expected error for 2024101. The focused Spark 3.5.7 GPU run passed.
| "MM-yyyy", | ||
| "MM/dd/yyyy", | ||
| "MM-dd-yyyy", | ||
| "yyyyMMdd", |
There was a problem hiding this comment.
On a separate note this list is also used by date_format, not just parsing. I don't know if we've verified that the reverse direction date_format(timestamp, 'yyyyMMdd') matches Spark.
There was a problem hiding this comment.
Verified and covered in 18bd197. yyyyMMdd is now included in the direct date_format parity matrix for both Date and Timestamp inputs, including the runtime-fallback and timezone-rule cases. The focused Spark 3.5.7 GPU run selected 25 yyyyMMdd cases: 24 passed and the Spark-4-only case skipped, with no failures or errors.
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
|
build |
1 similar comment
|
build |
Fixes #15588.
Description
The yyyyMMdd format currently falls back to CPU under the CORRECTED time parser policy unless incompatible date formats are enabled. The customer workload described in #15588 was 14.9% faster when the existing GPU path was forced.
This change:
Local verification:
Checklists
Documentation
Testing
(Please provide the names of the existing tests in the PR description.)
Performance