Skip to content

chore: fallback for spark.sql.mapKeyDedupPolicy == LAST_WIN#4863

Merged
comphead merged 5 commits into
apache:mainfrom
comphead:map_last_win
Jul 9, 2026
Merged

chore: fallback for spark.sql.mapKeyDedupPolicy == LAST_WIN#4863
comphead merged 5 commits into
apache:mainfrom
comphead:map_last_win

Conversation

@comphead

@comphead comphead commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Narrows the spark.sql.mapKeyDedupPolicy divergence for map_from_arrays and map_from_entries reported in #4680

Rationale for this change

Spark's ArrayBasedMapBuilder (used by both expressions) honors two dedup modes:

  • EXCEPTION (default): raise [DUPLICATED_MAP_KEY] on duplicate keys.
  • LAST_WIN: keep the last occurrence.

Comet is currently pinned to datafusion-spark 54.0.0, whose map_from_arrays / map_from_entries implement neither mode — duplicates simply pass through. That means:

  • Under EXCEPTION (the default) with duplicate keys, Comet silently keeps duplicates instead of erroring — a data divergence tracked separately.
  • Under LAST_WIN, Comet's output shape disagrees with Spark's per-key dedup — a config-driven divergence we can detect at plan time.

Upstream apache/datafusion#21720 wires both modes into datafusion-spark, but that fix has not yet landed in a release Comet pins to.

What changes are included in this PR?

  • Add MapKeyDedupPolicySupport in serde/maps.scala — one helper checks spark.sql.mapKeyDedupPolicy == LAST_WIN.
  • CometMapFromArrays and CometMapFromEntries return Incompatible when LAST_WIN is active, so planning falls back to Spark for that config only. EXCEPTION (default) continues to run natively.
  • Sibling expressions (CreateMap, MapConcat, TransformKeys, ...) already delegate to Spark's doGenCode via CometCodegenDispatch and inherit dedup semantics for free — no gate needed.

Are these changes tested?

Yes — new SQL file tests exercise the fallback with spark.sql.mapKeyDedupPolicy=LAST_WIN for both expressions, on literal and column inputs:

  • spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy.sql
  • spark/src/test/resources/sql-tests/expressions/map/map_from_entries_dedup_policy.sql

The map_from_entries test also disables the codegen dispatcher so the incompat branch surfaces as a genuine Spark fallback rather than in-pipeline codegen.

Follow-up

When Comet bumps to a datafusion-spark release containing #21720, this gate can be replaced by propagating spark.sql.mapKeyDedupPolicydatafusion.spark.map_key_dedup_policy and removing the
Incompatible branch. str_to_map gets the same upgrade for free.

Comment on lines +98 to +100
def isDefault: Boolean =
SQLConf.get.getConf(SQLConf.MAP_KEY_DEDUP_POLICY) ==
SQLConf.MAP_KEY_DEDUP_POLICY.defaultValueString

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Checking for default vs non-default value seems brittle. What if the default changes in a later Spark version? Shouldn't we check explicitly for EXCEPTION vs LAST_WIN?

@comphead comphead changed the title chore: fallback for non-default spark.sql.mapKeyDedupPolicy chore: fallback for spark.sql.mapKeyDedupPolicy == LAST_WIN Jul 8, 2026
Comment thread spark/src/main/scala/org/apache/comet/serde/maps.scala Outdated

@andygrove andygrove left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @comphead

@comphead comphead merged commit 1142c3a into apache:main Jul 9, 2026
137 of 138 checks passed
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.

3 participants