feat: Add Spark-dialect map_from_arrays to Velox - #18630
Open
emilysun201309 wants to merge 1 commit into
Open
Conversation
Summary: `map_from_arrays` is a Spark-only SQL name, but Velox bound it to the Presto implementation class `MapFunction<AllowDuplicateKeys=true>` (`prestosql/Map.cpp`). That variant skips the duplicate-key check entirely: it neither throws nor dedupes, emitting a MapVector that physically contains both entries for a repeated key. The result is not a valid map, and `element_at` then reads whichever entry it hits first. This adds `MapFromArraysFunction` in `sparksql/Map.cpp`, mirroring Spark's `ArrayBasedMapBuilder`: - length mismatch and null keys throw, with Spark's wording - under EXCEPTION, a repeated key raises DUPLICATED_MAP_KEY - under LAST_WIN, the repeated key's value overwrites the value of its FIRST occurrence in place, so the key keeps its first position. Nulls follow Spark rather than Presto: only a top-level null key is rejected. Presto additionally rejects an indeterminate key — a complex key containing a null — which Spark accepts. `BaseVector::equalValueAt` compares with `kNullAsValue`, so dedup treats two such keys as equal, consistent with Spark. Deduplication orders keys by `BaseVector::compare`, for every key type rather than only the ones that cannot be hashed. `compare` already reports NaN equal to NaN and `-0.0` equal to `0.0`, which is the same equality dedup owes `equalValueAt`; a hashed fast path has to reproduce that agreement by hand, and keeping one strategy removes that obligation along with the type dispatch. Three choices keep the common paths cheap: - registration is stateful, so the dedup policy is resolved once when the expression is compiled rather than read from the query config on every batch. The previous per-`apply()` read was guarded on a possibly-null `queryCtx` and silently defaulted to LAST_WIN. - under EXCEPTION no row can shrink, so when both arguments are identity-mapped and their arrays agree on offset and size, the result references the input arrays directly — no entry index buffers and no dictionary wrap. - a constant key array is deduped once instead of per row; a row then writes only its value indices. **dedup logic and time complexity** | key type | Spark | Presto | this diff | | --- | --- | --- | --- | | atomic, non-binary | `HashMap`, O(n) | sort, O(n log n) | ordered, O(n log n) | | binary | `TreeMap`, O(n log n) | sort, O(n log n) | ordered, O(n log n) | | complex | `TreeMap`, O(n log n) | sort, O(n log n) | ordered, O(n log n) | The null-key scan is skipped when the key elements vector reports no nulls, and the fast-path predicate keeps the name, contract and asserts of `canTakeFastPath` in `prestosql/Map.cpp`, so the two read alike. `isNullAt` is virtual, so the guard removes a call per key on the common path. Differential Revision: D115361136
emilysun201309
requested review from
jinchengchenghh and
rui-mo
as code owners
August 21, 2026 22:36
✅ Deploy Preview for meta-velox canceled.
|
Contributor
|
@emilysun201309 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115361136. |
Selective Build Plan
Affected targets (20)Directly changed (3)
Transitively affected (17)
Unresolved (2)These files could not be mapped to any target; a full build may be needed if they are build-relevant.
Slow path • Graph generated from PR branch |
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
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.
Summary:
map_from_arraysis a Spark-only SQL name, but Velox bound it to the Prestoimplementation class
MapFunction<AllowDuplicateKeys=true>(
prestosql/Map.cpp). That variant skips the duplicate-key check entirely: itneither throws nor dedupes, emitting a MapVector that physically contains both
entries for a repeated key. The result is not a valid map, and
element_atthen reads whichever entry it hits first.
This adds
MapFromArraysFunctioninsparksql/Map.cpp, mirroring Spark'sArrayBasedMapBuilder:occurrence in place, so the key keeps its first position.
Nulls follow Spark rather than Presto: only a top-level null key is rejected.
Presto additionally rejects an indeterminate key — a complex key containing a
null — which Spark accepts.
BaseVector::equalValueAtcompares withkNullAsValue, so dedup treats two such keys as equal, consistent with Spark.Deduplication orders keys by
BaseVector::compare, for every key type ratherthan only the ones that cannot be hashed.
comparealready reports NaN equalto NaN and
-0.0equal to0.0, which is the same equality dedup owesequalValueAt; a hashed fast path has to reproduce that agreement by hand, andkeeping one strategy removes that obligation along with the type dispatch.
Three choices keep the common paths cheap:
expression is compiled rather than read from the query config on every batch.
The previous per-
apply()read was guarded on a possibly-nullqueryCtxandsilently defaulted to LAST_WIN.
and their arrays agree on offset and size, the result references the input
arrays directly — no entry index buffers and no dictionary wrap.
only its value indices.
dedup logic and time complexity
HashMap, O(n)TreeMap, O(n log n)TreeMap, O(n log n)The null-key scan is skipped when the key elements vector reports no nulls, and
the fast-path predicate keeps the name, contract and asserts of
canTakeFastPathinprestosql/Map.cpp, so the two read alike.isNullAtisvirtual, so the guard removes a call per key on the common path.
Differential Revision: D115361136