refactor: rename withInfo to withFallbackReason for clarity#4508
Merged
Conversation
Rename withInfo/withInfos/hasExplainInfo and EXTENSION_INFO to withFallbackReason/withFallbackReasons/hasFallbackReason and FALLBACK_REASONS to match their actual semantics (fallback reasons, not generic info). Also rename the private extensionInfo helper in ExtendedExplainInfo to fallbackReasons, and update the TreeNodeTag string from "CometExtensionInfo" to "CometFallbackReasons" so a future PR can reuse the old string for a distinct tag.
parthchandra
approved these changes
May 29, 2026
Contributor
parthchandra
left a comment
parthchandra
left a comment
There was a problem hiding this comment.
lgtm. Though you might want to address the one comment.
|
|
||
| object CometExplainInfo { | ||
| val EXTENSION_INFO = new TreeNodeTag[Set[String]]("CometExtensionInfo") | ||
| val FALLBACK_REASONS = new TreeNodeTag[Set[String]]("CometFallbackReasons") |
Contributor
There was a problem hiding this comment.
Looks like CometMetricsListener is still referencing CometExplainInfo.EXTENSION_INFO. CometMetricsListener is not being used (?) but this might break CI
# Conflicts: # spark/src/main/scala/org/apache/comet/serde/strings.scala # spark/src/main/scala/org/apache/comet/serde/structs.scala
Apply rename consistently across the serde layer per review feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Part of #4006. This is a preparatory refactor that does not close the issue on its own; a follow-on PR adds the new informational message channel.
Rationale for this change
Comet uses
withInfoto tag plan nodes, but the method and its backingEXTENSION_INFOtag actually mean "this node falls back to Spark": the tag is read by the planning rules as a fallback signal, and a code comment already warns against using it for anything that is not a fallback reason. The name does not match the semantics, which is confusing and blocks adding a genuine informational channel (the goal of #4006). Renaming the fallback path frees thewithInfoname for that future use.What changes are included in this PR?
A pure, behavior-preserving rename across the
sparkmodule:withInfo/withInfos->withFallbackReason/withFallbackReasonshasExplainInfo->hasFallbackReasonextensionInfo->fallbackReasonsCometExplainInfo.EXTENSION_INFO->CometExplainInfo.FALLBACK_REASONS(the tag string also changes fromCometExtensionInfotoCometFallbackReasons)All call sites, import selectors, version-specific shims, and scaladoc are updated. No behavior changes: the same messages are still logged under
COMET_LOG_FALLBACK_REASONS, accumulated and rolled up the same way, and rendered as[COMET: ...]in extended explain output.How are these changes tested?
Covered by existing tests, which pass unchanged (the
withInfounit test inCometExpressionSuiteis renamed towithFallbackReasonand continues to assert the same behavior).scalastyle:checkpasses.