Copilot commented on code in PR #2071:
URL: https://github.com/apache/auron/pull/2071#discussion_r2889203613
##########
dev/auron-it/src/main/scala/org/apache/auron/integration/comparison/PlanStabilityChecker.scala:
##########
@@ -44,6 +44,11 @@ class PlanStabilityChecker(
def validate(test: QueryExecutionResult): Boolean = {
if (!isSupported) return true
+ if ("q54" == test.queryId) {
+ println(s"[PlanCheck] Skipping plan check for query ${test.queryId} due
to known non-determinism.")
+ return true
+ }
+
if (regenGoldenFiles) {
generatePlanGolden(test.queryId, test.plan)
} else if (planCheck) {
Review Comment:
The early return for `q54` runs before both `regenGoldenFiles` and
`planCheck`, so it also skips golden plan generation for q54. This makes it
impossible to regenerate a new golden file for q54 (e.g., if the
non-determinism is later fixed). Consider gating this skip to only apply when
`planCheck` is enabled (comparison mode), or otherwise allow regen to proceed
while still skipping the stability assertion.
```suggestion
if (regenGoldenFiles) {
generatePlanGolden(test.queryId, test.plan)
} else if (planCheck) {
if ("q54" == test.queryId) {
println(s"[PlanCheck] Skipping plan check for query ${test.queryId}
due to known non-determinism.")
return true
}
```
##########
dev/auron-it/src/main/scala/org/apache/auron/integration/comparison/PlanStabilityChecker.scala:
##########
@@ -44,6 +44,11 @@ class PlanStabilityChecker(
def validate(test: QueryExecutionResult): Boolean = {
if (!isSupported) return true
+ if ("q54" == test.queryId) {
+ println(s"[PlanCheck] Skipping plan check for query ${test.queryId} due
to known non-determinism.")
Review Comment:
The skip message says "known non-determinism" but doesn’t reference the
tracking issue or the scope (Spark 3.5). Including the issue ID (e.g., #2049)
and/or the shimVersion in the message would make it easier to understand later
why this special-case exists and when it can be removed.
```suggestion
println(s"[PlanCheck] Skipping plan check for query ${test.queryId} on
${Shims.get.shimVersion} due to known non-determinism (see issue #2049).")
```
--
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]
