andygrove opened a new issue, #93:
URL: https://github.com/apache/datafusion-spark/issues/93
This implements the design proposed in #16.
### Design
1. **Expressions take named behavior flags, never a Spark version.** For
example, `SparkArrayInsert::with_legacy_negative_index` (#70),
`legacy_size_of_null` (#90), or `decimal_zero_any_exponent` in the cast port
(#88).
2. **One module derives each flag's default from a full `major.minor.patch`
Spark version.** Spark backports behavior changes to patch releases, so a
`3.4`–`4.2` enum is not precise enough:
- `spark.sql.legacy.negativeIndexInArrayInsert` was added in 3.4.2.
- SPARK-60118 (casting a zero string with any exponent to decimal) first
shipped in 4.0.5, 4.1.4 and 4.2.1.
Each entry cites its SPARK JIRA and the first release in each line that
includes it, like Comet's
[`SparkFixes.scala`](https://github.com/apache/datafusion-comet/blob/main/spark/src/main/scala/org/apache/comet/serde/SparkFixes.scala).
3. **Where Spark has a setting for a behavior, the flag mirrors that
setting.** It becomes a key in `SparkOptions` (#92) whose default comes from
the version, and `SET` overrides it. For example,
`spark.sql.legacy.javaCharsets` did not exist before 4.0, so its effective
default is `true` for 3.x and `false` for 4.x (#46).
4. **Two ways in:**
- In a DataFusion session, the UDF resolves its flags from
`spark.version` plus any explicit setting, in `invoke_with_args`, or in
`with_updated_config` when the return type depends on the flag.
- Comet passes the flags directly, because its JVM side knows the exact
running version and the real `SQLConf`.
### Tasks
- [ ] `SparkVersion { major, minor, patch }`, parsed from a string such as
`4.0.5`, defaulting to the crate's baseline (4.2.0)
- [ ] `spark.version` in `SparkOptions`, which depends on #92
- [ ] The flag-derivation module, with unit tests for each entry
- [ ] Document the convention for adding a version-dependent behavior in
`CONTRIBUTING.md` or the audit skill
- [ ] Use `SET spark.version = ...` in sqllogictest files to cover
non-default versions, which partly addresses #24
### Behaviors waiting on this
| Function | Flag | Issue |
|---|---|---|
| `size` | `spark.sql.legacy.sizeOfNull` (with ANSI) | #90 |
| `elt` | `spark.sql.function.eltOutputAsString` | #91 |
| `encode` | `spark.sql.legacy.javaCharsets`,
`spark.sql.legacy.codingErrorAction` (3.x vs 4.x) | #46 |
| `hash`, `xxhash64` | `spark.sql.legacy.allowHashOnMapType` | #69 |
| `next_day` | error class differs between 3.5 and 4.x, with no setting |
#25 |
| `array_insert` | `spark.sql.legacy.negativeIndexInArrayInsert` | #70 |
| `format_string`, `concat`, `array()` | legacy settings whose defaults
already match | #94 |
| `CAST` | `is_spark4_plus`, `decimal_zero_any_exponent`,
`decimal_skip_integral_digit_limit` | #88 |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
