docs: add compatibility documentation to all expressions#4067
Merged
andygrove merged 18 commits intoapache:mainfrom Apr 24, 2026
Merged
docs: add compatibility documentation to all expressions#4067andygrove merged 18 commits intoapache:mainfrom
andygrove merged 18 commits intoapache:mainfrom
Conversation
Wire GenerateDocs to emit incompatibility and unsupported notes into each expression compatibility page, driven by getIncompatibleReasons and getUnsupportedReasons on the serde traits. Add matching defaults to CometAggregateExpressionSerde so aggregate.md is covered too. Fix CometDateFormat.getUnsupportedReasons formatting.
Move hand-written incompatibility and unsupported notes for CollectSet, Average, SortArray, TruncTimestamp, and StructsToJson from the per-category markdown pages into the corresponding serde via getIncompatibleReasons / getUnsupportedReasons, so GenerateDocs drives the compatibility guide from a single source of truth. Clarify in the trait scaladoc that reasons should be written in Markdown.
Create compatibility/expressions/math.md with an EXPR_COMPAT marker block and wire it to QueryPlanSerde.mathExpressions in GenerateDocs so CometAbs's unsupported-reason note surfaces in the guide. Add math to the expressions toctree.
Satisfy scalafix DisableSyntax.noExplicitPublicVal by annotating CometHour.incompatReason and CometAbs.unsupportedReason as `: String`.
… methods Add guidance to the contributor guide covering the new documentation methods on CometExpressionSerde. Also simplify CometDateFormat's getUnsupportedReasons to list only the supported Spark format keys.
…ion serdes - Remove getSupportLevel override from CometArrayAppend (always returned Compatible, which is the default) - Add getIncompatibleReasons() to all always-Incompatible expression serdes - Add getIncompatibleReasons() and/or getUnsupportedReasons() to all conditional expression serdes that were missing them - Add compatibility guide pages for string, map, and misc expression categories - Register new categories in GenerateDocs so content is auto-generated Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…for references Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
andygrove
changed the title
# Conflicts: # docs/source/contributor-guide/adding_a_new_expression.md # docs/source/user-guide/latest/compatibility/expressions/index.md # spark/src/main/scala/org/apache/comet/GenerateDocs.scala # spark/src/main/scala/org/apache/comet/serde/arrays.scala
…Notes Remove Spark-Compatible? and Compatibility Notes columns from expressions.md; those details now live in the generated Compatibility Guide. Add getCompatibleNotes() to CometExpressionSerde and CometAggregateExpressionSerde for differences that are always present and do not require opting in via allowIncompatible, rendered as a distinct section in the compatibility guide. Backfill reasons in serdes that previously only appeared in expressions.md.
# Conflicts: # docs/source/user-guide/latest/expressions.md # spark/src/main/scala/org/apache/comet/serde/arrays.scala
comphead
reviewed
Apr 24, 2026
| | BitXorAgg | | | ||
| | BoolAnd | `bool_and` | | ||
| | BoolOr | `bool_or` | | ||
| | CollectSet | | |
Contributor
There was a problem hiding this comment.
we prob need to address those gaps later. for example count, corr, collect_set supported and have sql expression
comphead
reviewed
Apr 24, 2026
| | UnscaledValue | Yes | | | ||
| | Expression | | ||
| | ---------------------------- | | ||
| | Alias | |
Contributor
There was a problem hiding this comment.
why some time we have SQL column and some times not?
Member
Author
There was a problem hiding this comment.
no good reason - do you think it is worth keeping sql column?
Member
Author
There was a problem hiding this comment.
maybe this doc does not need tables now and can just be simple lists
comphead
reviewed
Apr 24, 2026
|
|
||
| object CometFirst extends CometAggregateExpressionSerde[First] { | ||
|
|
||
| override def getCompatibleNotes(): Seq[String] = Seq( |
Contributor
There was a problem hiding this comment.
whats the difference between getCompatibleNotes and getIncompatibleReasons and getUnsupportedReasons. I can think of diff between 2 and 3, but 1 is confusing
Member
Author
There was a problem hiding this comment.
getCompatibleNotesis for compatibility issues that we decided to accept and still accelerate the expressiongetIncompatibleReasonsis for compatibility issues that we decided to fall back for and have user opt-in
comphead
reviewed
Apr 24, 2026
Contributor
comphead
left a comment
comphead
left a comment
There was a problem hiding this comment.
Thanks @andygrove this is great, finally all incompats in a single place
parthchandra
approved these changes
Apr 24, 2026
Contributor
parthchandra
left a comment
parthchandra
left a comment
There was a problem hiding this comment.
Looks good from my side.
Member
Author
|
Merged. Thanks @parthchandra @comphead |
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
3 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.
What issue does this close?
N/A
Rationale for this change
This PR documents the current state. There were some surprises. There is follow on issue #4074 to address those.
How is this tested
Manually