felipepessoto commented on issue #12779:
URL: https://github.com/apache/gluten/issues/12779#issuecomment-5301663512
## Correction / nuance on `TypeWideningInsertSchemaEvolutionBasicSuite`
Comparing the pinned `v4.2.0` test against `delta-io/delta@master`:
**v4.2.0** (what this CI pipeline actually runs):
```scala
withSQLConf(SQLConf.STORE_ASSIGNMENT_POLICY.key ->
StoreAssignmentPolicy.LEGACY.toString,
DeltaSQLConf.DELTA_ALLOW_AUTOMATIC_WIDENING.key -> "never") {
testCase.additionalValuesDF.write...insertInto(...) // no error tolerance
}
assert(readDeltaTable(tempPath).schema("value").dataType ===
testCase.fromType)
```
**master** (later Delta versions):
```scala
withSQLConf(DeltaSQLConf.DELTA_ALLOW_AUTOMATIC_WIDENING.key -> "never") {
mayOverflow { // <-- tolerates
overflow now
testCase.additionalValuesDF.write...insertInto(...)
}
}
```
`TypeWideningTestMixin.mayOverflow` (added upstream after v4.2.0) explicitly
swallows a write failure whose error class is `CAST_OVERFLOW_IN_TABLE_INSERT`,
`CAST_OVERFLOW`, or `DELTA_CAST_OVERFLOW_IN_TABLE_WRITE`:
```scala
protected def mayOverflow(write: => Unit): Unit = {
val allowedConditions = Set(
"CAST_OVERFLOW_IN_TABLE_INSERT", "CAST_OVERFLOW",
"DELTA_CAST_OVERFLOW_IN_TABLE_WRITE")
try { write } catch {
case e: Exception if isAllowedOverflow(e) =>
}
}
```
So **upstream Delta itself later acknowledged this write can legitimately
overflow/throw** and added tolerance for it -- that fix just hasn't been
backported to the `v4.2.0` tag this pipeline pins. On `v4.2.0`, the write is
still expected to succeed silently (no widening, no error), so Gluten's throw
is a real gap against that specific pinned tag, but not a genuine "new"
correctness bug relative to where upstream Delta test expectations have since
moved.
This does **not** change the analysis for `ImplicitMergeCastingSuite` /
`ImplicitStreamingMergeCastingSuite`: their `master`-branch source is unchanged
from `v4.2.0` for these tests -- `expectLegacyCastingBehaviour(sqlConfig)` is
still `true` for the failing config, and the test body still just calls
`sql(mergeCommand)` with zero exception tolerance in both versions. Those two
remain a case of Gluten throwing where no exception should occur at all, in
both the pinned and current upstream test suites.
Generated-by: Claude claude-sonnet-5 (GitHub Copilot CLI)
--
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]
