Feat: add support for `elt` expression #3269

kazantsev-maksim · 2026-01-25T13:48:30Z

Which issue does this PR close?

N/A

Rationale for this change

What changes are included in this PR?

Added native Comet support for Spark's elt function
Uses the SparkElt implementation from the datafusion-spark crate

How are these changes tested?

Added unit test in CometStringExpressionSuite.scala
Added new benchmark test in CometStringExpressionBenchmark.scala

Microbenchmark result:

This reverts commit 768b3e9.

kazantsev-maksim · 2026-01-25T14:09:43Z

spark/src/main/scala/org/apache/comet/serde/strings.scala

+object CometElt extends CometScalarFunction[Elt]("elt") {
+
+  override def getSupportLevel(expr: Elt): SupportLevel = {
+    if (expr.failOnError) {


TODO: https://github.com/apache/datafusion/blob/main/datafusion/spark/src/function/string/elt.rs#L128

coderfender · 2026-01-27T07:23:02Z

spark/src/main/scala/org/apache/comet/serde/strings.scala

+object CometElt extends CometScalarFunction[Elt]("elt") {
+
+  override def getSupportLevel(expr: Elt): SupportLevel = {
+    if (expr.failOnError) {


Thank you for the PR @kazantsev-maksim .Minor nitpick : We generally add ANSI mode not supported to make it clear to the user. I can work in enabling support for ANSI mode once this PR is merged as well

Thanks, fixed

coderfender · 2026-01-27T07:23:12Z

spark/src/main/scala/org/apache/comet/serde/strings.scala

+
+  override def getSupportLevel(expr: Elt): SupportLevel = {
+    if (expr.failOnError) {
+      return Unsupported(Some("failOnError=true is not supported"))


Same as above

coderfender · 2026-01-27T07:24:16Z

spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala

+        sql(s"SELECT elt(cast(null as int), ${schema.fieldNames.mkString(",")}) FROM t1"))
+      checkSparkAnswerMaybeThrows(sql("SELECT elt(1) FROM t1")) match {
+        case (Some(spark), Some(comet)) =>
+          assert(spark.getMessage.contains("WRONG_NUM_ARGS.WITHOUT_SUGGESTION"))


minor : perhaps a good idea to capture the error msg "WRONG_NUM_ARGS.WITHOUT_SUGGESTION" in a string ?

Thanks, fixed

coderfender · 2026-01-27T07:25:23Z

spark/src/test/scala/org/apache/spark/sql/benchmark/CometStringExpressionBenchmark.scala

    StringExprConfig("trim", "select trim(c1) from parquetV1Table"),
-    StringExprConfig("upper", "select upper(c1) from parquetV1Table"))
+    StringExprConfig("upper", "select upper(c1) from parquetV1Table"),
+    StringExprConfig("elt", "select elt(2, c1, c1) from parquetV1Table"))


thank you for adding this in the benchmark setup

coderfender

Thank you. Left minor comments

coderfender · 2026-01-27T07:35:25Z

spark/src/main/scala/org/apache/comet/serde/strings.scala

+  override def getSupportLevel(expr: Elt): SupportLevel = {
+    if (expr.failOnError) {
+      return Unsupported(Some("failOnError=true is not supported"))
+    }


We also might want to check if the inputs are a number and an array to make sure we terminate early in case the user provides malformed input

Relevant examples : CometLength , CometRPad , CometLPad

The first argument can be a numeric string - '2', it seems we can't cover all cases here.

I think we can do a selective support for now (say Int, String) inputs for now and file an issue for further enhancements down the line. WDYT @kazantsev-maksim ?

I've added it. @coderfender Can you please see?

codecov-commenter · 2026-01-27T16:37:48Z

Codecov Report

❌ Patch coverage is 26.31579% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.91%. Comparing base (f09f8af) to head (8e3f482).
⚠️ Report is 907 commits behind head on main.

Files with missing lines	Patch %	Lines
...rc/main/scala/org/apache/comet/serde/strings.scala	17.64%	10 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3269      +/-   ##
============================================
+ Coverage     56.12%   59.91%   +3.78%     
- Complexity      976     1462     +486     
============================================
  Files           119      175      +56     
  Lines         11743    16185    +4442     
  Branches       2251     2687     +436     
============================================
+ Hits           6591     9697    +3106     
- Misses         4012     5136    +1124     
- Partials       1140     1352     +212

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

# Conflicts: # native/core/src/execution/jni_api.rs

coderfender · 2026-01-29T22:46:09Z

spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala


+  private val WRONG_NUM_ARGS_WITHOUT_SUGGESTION_EXCEPTION_MSG =
+    "[WRONG_NUM_ARGS.WITHOUT_SUGGESTION] The `elt` requires > 1 parameters but the actual number is 1."
+


minor : given that this message is only applicable for your expression test , may be we could move it inside the test itself ?

coderfender · 2026-01-29T22:55:06Z

spark/src/main/scala/org/apache/comet/serde/strings.scala

+          .mkString(", ")
+      return Unsupported(
+        Some(
+          s"Parameters 2 and onwards require the string type, but contains: $unsupportedTypes"))


minor : we could use StringType.toSQL to get SQL test for StringType to make it easier for user's to parse error messages

minor : might be a good idea to not do distinct here ..
Some of the options I could think of are

Exhaustive error message : elt(1 , "apple", "banana", 10 , true , "StringType") will produce Parameters 2 and onwards require the string type, but contains: StringType, IntegerType , BooleanType and could keep the user guessing . However, if we remove distinct and preserve order , it might be Parameters 2 and onwards require the string type, but datatypes supplied are StringType, IntegerType , BooleanType , StringType

Break at first non string type and return unsupported to reduce redundancy

But we could do that in a follow up PR by raising an issue on github too

coderfender

left minor comments

coderfender · 2026-01-29T22:59:52Z

spark/src/main/scala/org/apache/comet/serde/strings.scala

+          .mkString(", ")
+      return Unsupported(
+        Some(
+          s"Parameters 2 and onwards require the string type, but contains: $unsupportedTypes"))


minor : might be a good idea to not do distinct here ..
Some of the options I could think of are

Exhaustive error message : elt(1 , "apple", "banana", 10 , true , "StringType") will produce Parameters 2 and onwards require the string type, but contains: StringType, IntegerType , BooleanType and could keep the user guessing . However, if we remove distinct and preserve order , it might be Parameters 2 and onwards require the string type, but datatypes supplied are StringType, IntegerType , BooleanType , StringType

Break at first non string type and return unsupported to reduce redundancy

But we could do that in a follow up PR by raising an issue on github too

coderfender · 2026-01-29T23:03:33Z

spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala

+          assert(comet.getMessage.contains(WRONG_NUM_ARGS_WITHOUT_SUGGESTION_EXCEPTION_MSG))
+        case (spark, comet) =>
+          fail(
+            s"Expected Spark and Comet to throw exception, but got\nSpark: $spark\nComet: $comet")


it would be great if you can add more tests to cover other code paths

Non string data types from pos 2

ANSI mode should fall back successfully to Spark

Index out of range and make sure we return NULL

Verify that first arg being Int is being enforced properly (perhaps pass a string / long type input as a negative test)

andygrove · 2026-01-30T18:23:29Z

spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala

+        "idx",
+        lit(Random.shuffle(indexes ++ edgeCasesIndexes).headOption.getOrElse(-1)))
+        .createOrReplaceTempView("t1")
+      checkSparkAnswerAndOperator(


It would also be worth adding a literal test with constant folding disabled, something like:

withSQLConf(SQLConf.OPTIMIZER_EXCLUDED_RULES.key -> "org.apache.spark.sql.catalyst.optimizer.ConstantFolding") { checkSparkAnswerAndOperator("SELECT elt(2, 'a', 'b', 'c')")

It is totally fine to fall back to Spark if all inputs are literal.

Kazantsev Maksim and others added 19 commits December 14, 2025 16:24

impl map_from_entries

768b3e9

Revert "impl map_from_entries"

c68c342

This reverts commit 768b3e9.

Merge branch 'apache:main' into main

d887555

Merge branch 'apache:main' into main

231aa90

Merge branch 'apache:main' into main

9500bbb

Merge branch 'apache:main' into main

9577481

Merge branch 'apache:main' into main

3791557

Merge branch 'apache:main' into main

7c2f082

Merge branch 'apache:main' into main

609a605

Merge branch 'apache:main' into main

a151b2c

Merge branch 'apache:main' into main

ad3e7f5

Merge branch 'apache:main' into main

ea92e4b

Merge branch 'apache:main' into main

8dfeca3

Merge branch 'apache:main' into main

559741e

Merge branch 'apache:main' into main

ebda14e

Merge branch 'apache:main' into main

408152e

Merge branch 'apache:main' into main

d7857b2

Feat: impl elt function

63fb715

Added micro benchmark test

4b07cc4

kazantsev-maksim commented Jan 25, 2026

View reviewed changes

Fix fmt

0f518f6

coderfender reviewed Jan 27, 2026

View reviewed changes

Kazantsev Maksim added 2 commits January 27, 2026 21:26

Fix PR issues

1604dc6

Fix PR issues

fc22ee7

kazantsev-maksim and others added 3 commits January 29, 2026 21:59

Merge branch 'apache:main' into main

aef41be

Merge remote-tracking branch 'origin/main' into elt_func

70c400b

# Conflicts: # native/core/src/execution/jni_api.rs

Fix PR issues

8e3f482

coderfender reviewed Jan 29, 2026

View reviewed changes

andygrove reviewed Jan 30, 2026

View reviewed changes


		private val WRONG_NUM_ARGS_WITHOUT_SUGGESTION_EXCEPTION_MSG =
		"[WRONG_NUM_ARGS.WITHOUT_SUGGESTION] The `elt` requires > 1 parameters but the actual number is 1."

Feat: add support for elt expression #3269

Are you sure you want to change the base?

Feat: add support for elt expression #3269

Conversation

kazantsev-maksim commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderfender left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderfender Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderfender left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Feat: add support for `elt` expression #3269

Feat: add support for `elt` expression #3269

kazantsev-maksim commented Jan 25, 2026 •

edited

Loading

coderfender left a comment •

edited

Loading

coderfender Jan 27, 2026 •

edited

Loading

codecov-commenter commented Jan 27, 2026 •

edited

Loading