What happens
The Gluten-authored unhex case has two assertions commented out, with nothing saying whether they are known failures or leftovers. In gluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala, inside testGluten("unhex") at :283:
// checkEvaluation(Unhex(Literal("GG")), null)
...
// checkEvaluation(Unhex(Literal("三重的")), null)Both are the "invalid hex input returns null" cases. The same two lines are commented out in all five version modules.
Why this is probably a ClickHouse limitation, not a Velox one
Worth recording, because it points at the fix. Spark's own MathExpressionsSuite."unhex" contains both assertions, and none of the four surviving modules' VeloxTestSettings excludes it (nor does spark33's). So the vanilla case, null assertions included, has been running on Velox all along and passing. Whatever these two lines were commented out for, it was not Velox.
ClickHouse is the other side: every version excludes the vanilla case (.exclude("unhex") on 3.3/3.4, .excludeCH("unhex") on 3.5/4.0/4.1), so the Gluten copy is what actually runs there. That makes ClickHouse the plausible reason the lines are commented, and it means the Gluten copy on Velox is currently a strict subset of a vanilla case that already runs next to it.
Why it matters
As written, the two lines are dead text that rots. Worse, the arrangement is fragile: if someone later adds .exclude("unhex") to VeloxTestSettings following the usual convention for a rewritten case, the null coverage disappears on Velox and Gluten - unhex stays green, because it does not test those inputs.
Suggested fix
Uncomment both lines. If ClickHouse cannot pass them, add excludeGlutenTest("unhex") to the ClickHouse settings so "CH does not check this" is written down instead of implied by a comment. Either way the case should say which backend the restriction belongs to.
Surfaced by Copilot on #12840 over three review rounds; the backend analysis above comes from a review pass on that PR.
What happens
The Gluten-authored
unhexcase has two assertions commented out, with nothing saying whether they are known failures or leftovers. Ingluten-ut/spark35/src/test/scala/org/apache/spark/sql/catalyst/expressions/GlutenMathExpressionsSuite.scala, insidetestGluten("unhex")at:283:Both are the "invalid hex input returns null" cases. The same two lines are commented out in all five version modules.
Why this is probably a ClickHouse limitation, not a Velox one
Worth recording, because it points at the fix. Spark's own
MathExpressionsSuite."unhex"contains both assertions, and none of the four surviving modules'VeloxTestSettingsexcludes it (nor does spark33's). So the vanilla case, null assertions included, has been running on Velox all along and passing. Whatever these two lines were commented out for, it was not Velox.ClickHouse is the other side: every version excludes the vanilla case (
.exclude("unhex")on 3.3/3.4,.excludeCH("unhex")on 3.5/4.0/4.1), so the Gluten copy is what actually runs there. That makes ClickHouse the plausible reason the lines are commented, and it means the Gluten copy on Velox is currently a strict subset of a vanilla case that already runs next to it.Why it matters
As written, the two lines are dead text that rots. Worse, the arrangement is fragile: if someone later adds
.exclude("unhex")toVeloxTestSettingsfollowing the usual convention for a rewritten case, the null coverage disappears on Velox andGluten - unhexstays green, because it does not test those inputs.Suggested fix
Uncomment both lines. If ClickHouse cannot pass them, add
excludeGlutenTest("unhex")to the ClickHouse settings so "CH does not check this" is written down instead of implied by a comment. Either way the case should say which backend the restriction belongs to.Surfaced by Copilot on #12840 over three review rounds; the backend analysis above comes from a review pass on that PR.