andygrove opened a new issue, #7300: URL: https://github.com/apache/datafusion-comet/issues/7300
### What is the problem the feature request solves? We want Comet to evaluate as many Spark built-in expressions as possible natively, by default, with results identical to Spark. This issue sets the priority order and tracks the work. Counts below come from the generated table in [Spark Expression Support](https://github.com/apache/datafusion-comet/blob/main/docs/source/user-guide/latest/expressions.md) on `main` at `a828088500` (454 listed functions): | State | Functions | What the user gets | |---|---|---| | Native | 190 | Rust, no JNI round trip | | Hybrid | 66 | A native path exists, but some inputs (or, for several, the default) route through the **JVM codegen dispatcher**; the notes column says which | | Codegen dispatch only | 58 | Spark's generated Java runs inside the Comet pipeline, one JNI round trip per batch | | Planned or falling back | 19 | The whole operator falls back to Spark | | No direct wire-up | 120 | Rewritten to another expression, folded to a literal, or handled at the operator level; the notes column has the real status | Beyond the table, the "Not currently planned" section of that page lists function families that fall back to Spark (sketches, geospatial, Avro and Protobuf, JVM reflection, UTF-8 validation, and a few others). #### Why this matters 1. **Dispatch is a bridge, not a destination.** It is exact by construction, but it evaluates per row in the JVM, allocates on the JVM heap outside Comet's memory pool, and costs a JNI round trip per batch. The 124 Hybrid and dispatch-only functions are the largest block of remaining speedup, though how much of each Hybrid function still dispatches varies. 2. **Aggregates, window functions and generators cannot be dispatched.** The dispatcher covers scalar expressions only, so a missing native aggregate or generator sends the whole operator back to Spark. Those gaps cost more than any dispatched scalar. 3. **Comet Native Executors (#7288) are strict-native.** A query either runs entirely natively or fails, and the strict-native check rejects `JvmScalarUdf`, which codegen dispatch emits. Every expression that depends on the dispatcher today is a query that cannot run on a native executor. That is why this epic is on the 2.0.0 milestone. #### Definition of done for an expression - It runs in Rust by default, with no codegen dispatch and no `allowIncompatible` opt-in required for ordinary inputs. - It matches Spark bit for bit on every supported Spark version (3.4 through 4.2), including ANSI mode, NULL and NaN/-0.0 handling (#6385), and error messages. - Inputs that really cannot match fall back, or dispatch, deliberately and visibly, with the reason in `getSupportLevel`. A narrow documented exception is acceptable; a quietly different answer is not. - It has SQL file tests (`docs/source/contributor-guide/sql-file-tests.md`), an entry in the per-category audit log under `docs/source/contributor-guide/expression-audits/`, and a regenerated row in the expression reference. ### Describe the potential solution #### How the order was chosen Items are ranked by: (1) the cost of today's path (whole-operator fallback, then dispatch), (2) how often the function appears in real Spark workloads, (3) how much one piece of work unblocks, and (4) how cheaply a bit-exact native path can be built (an upstream `datafusion-spark` kernel, or a well-specified algorithm). Reach is my editorial judgment, not telemetry. If you have workload evidence that moves an item, say so on this issue and we will move it. Higher tiers are not strictly "do first". Tier 2 contains four large pieces that can proceed in parallel with each other and with Tier 1. Tier 3 items are small, independent, and good first contributions. Child issues are filed as an item is picked up, not up front. When you pick an item, use the `implement-comet-expression`, `wire-datafusion-function` and `suggest-native-expression` skills under `.ai/skills/`, and link the child issue from the checkbox here. #### Tier 1: operators that fall back whole (no dispatcher rescue) Aggregates: - [ ] `kurtosis`, `skewness`: no native implementation - [ ] `percentile_disc`; `percentile` with an array of percentages or a frequency argument; `percentile_cont ... DESC` - [ ] `approx_percentile` on input types other than byte, short, int, long, float and double - [ ] `listagg` / `string_agg` with `WITHIN GROUP (ORDER BY ...)` and with `BinaryType` - [ ] `min`, `max`, `sum`, `avg`, `try_avg` over interval types (#5061) - [ ] `avg` / `mean` over `DECIMAL` with precision 28 or more, where fallback depends on the aggregate operator - [ ] `hll_sketch_agg`, `hll_union_agg`, `hll_sketch_estimate`, `hll_union`: make the native path the default (Spark 4.0+); needs a merge that is bit-identical to Spark's - [ ] `mode`: Spark breaks ties non-deterministically, so the default is fallback. Decide whether a tie rule exists that is always a valid Spark answer, or record fallback as permanent Window functions, generators and grouping operators: - [ ] `lag` / `lead` with a non-literal default (#4268), plus the other window gaps listed in `roadmap.md` (#4766, #4834, #4835) - [ ] `session_window` (#4785) - [ ] `inline`, `inline_outer`, `stack` (#5125) - [ ] `json_tuple` (#3160) Scalars with no path at all: - [ ] `raise_error`, `assert_true`: need Spark-compatible error propagation from native - [ ] Two-argument `ceil` / `floor`, and negative-scale decimal `/` and `div` (#7023) - [ ] Interval arithmetic that falls back whole: `*` and `/` on calendar and year-month intervals, `try_add` on datetime and interval, `abs` and unary minus on intervals (see #5061) #### Tier 2: four engines that each unblock a family These are large, but each replaces the dispatcher for several of the most frequently used functions. Suggested order is by reach. **2a. A native regex engine with Java semantics.** Today only an in-subset `rlike` literal runs natively by default; everything else dispatches because the Rust `regex` crate differs from `java.util.regex`. Options include translating a verified subset, a backtracking engine that follows Java, or both. The design decision belongs in the first child issue. - [ ] `rlike` / `regexp` / `regexp_like` (out-of-subset literals, non-literal and NULL patterns) - [ ] `regexp_replace`, `regexp_extract`, `regexp_extract_all` - [ ] `regexp_count`, `regexp_substr` (rewrite to the above), `regexp_instr` (no native path) - [ ] `split` (regex separators) **2b. Datetime patterns and time zones.** Port Spark's formatter and parser behavior, including the time-parser policy, ANSI mode and the session time zone. Tracked in part by #4418. - [ ] `date_format` (#2014), `from_unixtime`, `unix_timestamp`, `to_unix_timestamp` - [ ] `to_timestamp`, `to_date` and `try_*` forms with a format (`GetTimestamp`) - [ ] `make_timestamp`, `make_timestamp_ltz`, `make_timestamp_ntz` and `try_*` - [ ] `convert_timezone`, `from_utc_timestamp`, `to_utc_timestamp`: native path for all time zone forms - [ ] `date_trunc`, `trunc`, `next_day` (collated `dayOfWeek`), `timestamp_seconds` (decimal, byte, short) **2c. JSON.** The native path differs for single-quoted JSON, unescaped control characters and trailing content. Decide per difference whether to match Spark's Jackson behavior or detect and defer. - [ ] `get_json_object` (nested wildcards differ, see the JSON audit), `json_array_length`, `json_object_keys` - [ ] `from_json`, `schema_of_json`, `to_json` (options, map and array inputs) **2d. Native higher-order functions.** `array_sort` and `filter` are native. The rest dispatch and interpret the lambda body per row on the JVM. DataFusion has native higher-order functions (`array_transform`, `array_filter`, `array_any_match`); whether their semantics match Spark is not yet known. See "Native Lambda Evaluation" in `roadmap.md`. - [ ] `transform`, `aggregate` / `reduce`, `exists`, `forall`, `zip_with` - [ ] `map_filter`, `map_zip_with`, `transform_keys`, `transform_values` #### Tier 3: individual scalars (small, independent, good first issues) Where `datafusion-spark` already ships a kernel (marked **upstream**), the first step is to check why Comet does not use it by default. Math and number formatting: - [ ] `round` and `bround` on float and double (`round` dispatches today), `pmod` (#6781), `positive` - [ ] `hypot` (**upstream**), `log1p`, `conv`, `nanvl`, `width_bucket` (**upstream**, already registered) - [ ] `format_number`, `to_number`, `try_to_number`, `to_char` / `to_varchar` on numbers - [ ] `abs` on interval types; `greatest` / `least` on collated strings Strings: - [ ] `lpad`, `rpad`: literal string input, column padding, binary input (#7245) - [ ] `concat_ws` with all-foldable arguments, `translate` (DataFusion works on graphemes, Spark on code points) - [ ] `elt` (**upstream**), `find_in_set`, `locate` / `position`, `overlay` - [ ] `format_string` (**upstream**), `printf` - [ ] `unbase64`, `to_binary` base64 and `utf-8` forms, `try_to_binary`, `mask` - [ ] `upper`, `lower`, `lcase`, `ucase`, `initcap` (#5353, #7242), `levenshtein`, `substring_index`, `trim` / `ltrim` / `rtrim` with a trim string - [ ] `like`, `ilike`, `in` (non-default collations and the remaining fallbacks) Datetime and intervals: - [ ] `add_months` (**upstream**), `months_between` - [ ] `make_interval` (#5131, #5279), `make_dt_interval` (**upstream**), `make_ym_interval` - [ ] `timestamp_micros`, `timestamp_millis`, `unix_micros`, `unix_millis`, `unix_seconds` - [ ] `timestampadd`, `timestampdiff`, `timediff`, and the TIME functions `time_diff`, `time_trunc`, `to_time`, `make_time` (Spark 4.1+, behind `spark.sql.timeType.enabled`) Collections: - [ ] `map`, `map_concat`, `map_from_arrays` (**upstream**), `map_from_entries` (**upstream**; float keys, `LAST_WIN`, binary), `str_to_map` (**upstream**; the `truncateForEmptyRegexSplit` legacy flag and collations) - [ ] `array_except`, `array_intersect`, `array_join`, `array_contains` (nested float elements), `arrays_overlap` - [ ] `sort_array` (other element types), `sequence` (date and timestamp) - [ ] `concat` and `reverse` on binary, array and struct elements; `array_position`, `array_distinct`, `array_union`, `flatten`, `shuffle` on binary, struct and map elements; `named_struct` with duplicate field names #### Tier 4: formats and new type families - [ ] XML: `xpath`, `xpath_boolean`, `xpath_double`, `xpath_float`, `xpath_int`, `xpath_long`, `xpath_number`, `xpath_short`, `xpath_string`, `from_xml`, `to_xml`, `schema_of_xml` - [ ] CSV: `from_csv`, `to_csv`, `schema_of_csv` (#6446, #6800) - [ ] Crypto: `aes_encrypt`, `aes_decrypt`, `try_aes_decrypt` - [ ] Variant: `parse_json`, `try_parse_json`, `variant_get`, `try_variant_get`, `is_variant_null`, `schema_of_variant`, `schema_of_variant_agg`, `to_variant_object` (#5438) - [ ] Interval type gaps (#5061) #### Tier 5: collation-aware evaluation Nearly every "non-`UTF8_BINARY` collated input routes through the JVM codegen dispatcher" note in the reference page traces to the same gap: no native collation support. This is large and its audience is narrow today, so it comes last, but it is the reason a number of Tier 3 rows cannot reach 100% native. - [ ] `collate` and collation-aware comparison, `GROUP BY` and joins (#2190) #### Tier 6: families currently listed as "not planned" This epic reopens the "Not currently planned" section of the reference page. The cheap ones come first because `datafusion-spark` already ships the kernels. - [ ] `is_valid_utf8`, `make_valid_utf8`, `validate_utf8`, `try_validate_utf8` (**upstream**: `is_valid_utf8`, `make_valid_utf8`) - [ ] `quote` (**upstream**), `bitmap_bit_position`, `bitmap_bucket_number`, `bitmap_count` (**upstream**), then `bitmap_construct_agg`, `bitmap_or_agg` - [ ] `sentences`, `histogram_numeric`, `version` - [ ] Sketches and approximate top-k: `kll_sketch_*`, `theta_*`, `count_min_sketch`, `approx_top_k*` (must be bit-identical, serialized sketches are visible to users) - [ ] Geospatial `st_*` (Spark 4.1) **Expected to stay on the JVM** (they invoke arbitrary JVM code or belong to the IO layer): `reflect`, `java_method`, `try_reflect`, Scala and Java UDFs, Python UDFs, `from_avro`, `to_avro`, `schema_of_avro`, `from_protobuf`, `to_protobuf`. Native Executors will need an answer for UDFs separately. #### Related work - #7288 Comet Native Executors (why this is on 2.0.0) - #4418 Implement all Spark date/time expressions - #4506 Provide codegen-dispatch implementations for Incompatible expressions so they never fall back by default (the reverse direction: this epic replaces the dispatch default with a native one) - #5572 Codegen-dispatch coverage audit - #5061 Complete interval type support - #5438 Native Variant support - #6385 Match Spark's -0.0 and NaN semantics systematically - #5128 Optimize native cast expression kernels - #5104 Replace hand-rolled native code with existing arrow-rs kernels - #6615 Move expression test coverage to Comet SQL tests #### Tracking progress The Implementation column of the expression reference is generated from the serde definitions, so the counts in the table above can be recomputed at any time. The goal for this epic is for the Hybrid and Codegen dispatch rows to reach zero, apart from the documented exceptions above. ### Additional context Counts and per-function notes come from `docs/source/user-guide/latest/expressions.md`. Some "No direct wire-up" rows are not simple rewrites: for example #5061 reports `make_dt_interval` and `make_ym_interval` running fully native where the table says codegen dispatch, so the first step of any Tier 3 item should be to check what runs today on a Parquet-backed (not constant-folded) input. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
