Skip to content

chore: promote array_insert to compatible and consolidate expression docs#4065

Merged
andygrove merged 2 commits intoapache:mainfrom
andygrove:array-insert-compatible
Apr 24, 2026
Merged

chore: promote array_insert to compatible and consolidate expression docs#4065
andygrove merged 2 commits intoapache:mainfrom
andygrove:array-insert-compatible

Conversation

@andygrove
Copy link
Copy Markdown
Member

@andygrove andygrove commented Apr 24, 2026

Which issue does this PR close?

Part of #4050

Rationale for this change

The array_insert expression was previously marked Incompatible, requiring spark.comet.expr.allowIncompatible=true to use. A recent audit against Spark 3.4.3, 3.5.8, and 4.0.1 found no behavioral differences beyond a cosmetic error-message mismatch when pos=0 (Comet throws a generic error while Spark raises INVALID_INDEX_OF_ZERO). That falls within the Compatible contract ("may have known differences in some specific edge cases that are unlikely to be an issue for most users"), so users no longer need to opt in to incompatible expressions for this function.

The expression audit log has also accumulated a single entry and lives in its own file. Folding that metadata into the main expression support doc keeps audit findings next to the coverage matrix, and moving that doc into the contributor guide makes it discoverable through the Sphinx toctree.

What changes are included in this PR?

  • Change CometArrayInsert.getSupportLevel from Incompatible(None) to Compatible().
  • Move docs/spark_expressions_support.md into docs/source/contributor-guide/ and add it to the contributor-guide toctree.
  • Merge the expression-audit-log.md contents into the support doc as per-version sub-bullets under array_insert, then delete the audit log.
  • Update the audit-comet-expression skill and adding_a_new_expression.md link to point at the new location.

How are these changes tested?

Covered by existing array_insert tests added during the audit. No new behavior is introduced; this PR only relaxes the support-level gate and reorganizes docs.

…docs

Promote `array_insert` from `Incompatible` to `Compatible` based on the
audit against Spark 3.4.3, 3.5.8, and 4.0.1. The only remaining
difference is a cosmetic error-message mismatch when `pos=0`, which
falls within the `Compatible` contract.

Move `docs/spark_expressions_support.md` into the contributor guide and
merge the separate expression audit log into it as per-version
sub-bullets under each audited expression.
object CometArrayInsert extends CometExpressionSerde[ArrayInsert] {

override def getSupportLevel(expr: ArrayInsert): SupportLevel = Incompatible(None)
override def getSupportLevel(expr: ArrayInsert): SupportLevel = Compatible()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have just removed the method, but don't want to wait for another CI run at this point. Will remove in a future PR along with others like this

| ArrayDistinct | Yes | |
| ArrayExcept | No | |
| ArrayFilter | Yes | Only supports case where function is `IsNotNull` |
| ArrayInsert | No | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @andygrove

@andygrove andygrove merged commit a80a3a8 into apache:main Apr 24, 2026
134 checks passed
@andygrove andygrove deleted the array-insert-compatible branch April 24, 2026 17:04
@andygrove
Copy link
Copy Markdown
Member Author

Thanks @comphead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants