malinjawi commented on code in PR #12647:
URL: https://github.com/apache/gluten/pull/12647#discussion_r4524604443
##########
backends-velox/src-delta33/main/scala/org/apache/spark/sql/delta/GlutenOptimisticTransaction.scala:
##########
@@ -91,53 +91,64 @@ class GlutenOptimisticTransaction(delegate:
OptimisticTransaction)
val empty2NullPlan =
convertEmptyToNullIfNeeded(queryExecution.executedPlan,
partitioningColumns, constraints)
- val maybeCheckInvariants = if (constraints.isEmpty) {
+ val rowInvariantPlan = if (constraints.isEmpty) {
// Compared to vanilla Delta, we simply avoid adding the invariant
checker
// when the constraint list is empty, to prevent the unnecessary
transitions
// from being added around the invariant checker.
empty2NullPlan
} else {
DeltaInvariantCheckerExec(empty2NullPlan, constraints)
}
Review Comment:
Yes to unwrap the `GlutenException` into a
`DeltaInvariantViolationException` I went with matching on the error message,
as you suggested:
- When the write fails, the transaction looks for the exact NOT NULL message
of each constraint the node checks. If it finds one, it rethrows Delta's
`DeltaInvariantViolationException`. Any other error is rethrown as is.
- There's a TODO to remove this once #13561 lands.
I also moved the check into its own node, `GlutenDeltaInvariantCheckerExec`,
placed where Delta puts `DeltaInvariantCheckerExec`, so the writer changes are
gone.
--
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]
