[SPARK-52770][CONNECT] Support `TIME` type in connect proto #51464

peter-toth · 2025-07-13T10:46:04Z

What changes were proposed in this pull request?

This PR adds TIME type support to Spark Connect.

Why are the changes needed?

TIME type is new in 4.1 and Spark Connect should support it.

Does this PR introduce any user-facing change?

Yes, it adds basic TIME type support.

How was this patch tested?

Added new UTs.

Was this patch authored or co-authored using generative AI tooling?

No.

MaxGekk · 2025-07-13T13:29:47Z

@dengziming Could you review this PR since you are working on similar one: #51462

dengziming

Thank you @peter-toth for this PR, I think we should add a test case for this change, and I don't think your test case makes sense, I duplicated it locally and it run smoothly even without this change, take a look at the image below.

dengziming · 2025-07-13T14:35:37Z

sql/connect/common/src/main/protobuf/spark/connect/types.proto

Should we make precision optional analogous to Decimal.

dongjoon-hyun

Thank you, @peter-toth .

dongjoon-hyun

BTW, the PR description looks wrong to me.

# How was this patch tested?

Manually:

scala> sql("SELECT TIME '12:13:14'").show()
+---------------+
|TIME '12:13:14'|
+---------------+
|       12:13:14|
+---------------+

As reported on the JIRA issue, show is handled by differently and works already. collect was the problem.

$ bin/spark-connect-shell --remote sc://localhost:15002
Welcome to
      ____              __
     / __/__  ___ _____/ /__
    _\ \/ _ \/ _ `/ __/  '_/
   /___/ .__/\_,_/_/ /_/\_\   version 4.1.0-preview1
      /_/

Type in expressions to have them evaluated.
Spark connect server version 4.1.0-preview1.
Spark session available as 'spark'.

scala> sql("SELECT TIME '12:13:14'").show()
+---------------+
|TIME '12:13:14'|
+---------------+
|       12:13:14|
+---------------+

scala> sql("SELECT TIME '12:13:14'").collect()
org.apache.spark.SparkException: org.apache.spark.sql.connect.common.InvalidPlanInput: [INTERNAL_ERROR] Does not support convert time(6) to connect proto types. SQLSTATE: XX000

dongjoon-hyun

Please add a test case like the above comment.

dengziming · 2025-07-14T05:19:12Z

The difference is that we need to return the result schema which is represented by DataType, when calling show the schema is converted to StringType so it will work well, whereas when calling collect we need TimeType, this can't be tested by the tests I provided before.

-- sql("SELECT TIME '12:13:14'").show()
root {
  show_string {
    input {
      sql {
        query: "SELECT TIME \'12:13:14\'"
      }
    }
    num_rows: 20
    truncate: 20
    vertical: true
  }
}

-- sql("SELECT TIME '12:13:14'").collect()
root {
  sql {
    query: "SELECT TIME \'12:13:14\'"
  }
}

peter-toth · 2025-07-14T05:34:39Z

@dongjoon-hyun, ahh indeed, I ran the wrong test. The proper type support needs a bit more changes.

The difference is that we need to return the result schema which is represented by DataType, when calling show the schema is converted to StringType so it will work well, whereas when calling collect we need TimeType, this can't be tested by the tests I provided before.

@dengziming, that's expected.

dongjoon-hyun

Thank you for updating the PR, @peter-toth .

peter-toth · 2025-07-14T14:38:15Z

Thank you for updating the PR, @peter-toth .

Sure. I will add some more tests and adjust the description as well, but I don't see yet what would be the best place for the sql("SELECT TIME '12:13:14'").collect() test.

dongjoon-hyun · 2025-07-14T14:38:53Z

sql/connect/common/src/main/protobuf/spark/connect/types.proto

    // UnparsedDataType
    Unparsed unparsed = 24;
+
+    Time time = 28;


Just for my understanding. Do you know where is 25, 26, 27, @peter-toth ?

Unfortunately these are not in order. A few lines up:

Variant variant = 25;

and down:

// Reserved for geometry and geography types reserved 26, 27;

dongjoon-hyun

+1, LGTM from my side. Thank you, @peter-toth .

cc @MaxGekk , @grundprinzip , @HyukjinKwon

HyukjinKwon

cc @MaxGekk @hvanhovell @grundprinzip FYI

LGTM

dengziming · 2025-07-15T02:12:01Z

sql/catalyst/src/main/java/org/apache/spark/sql/vectorized/ArrowColumnVector.java

      accessor = new TimestampAccessor(timeStampMicroTZVector);
    } else if (vector instanceof TimeStampMicroVector timeStampMicroVector) {
      accessor = new TimestampNTZAccessor(timeStampMicroVector);
+    } else if (vector instanceof TimeNanoVector timeNanoVector) {


Just a stupid question, when is these method used? why should we change arrow related method when adding connect dataType?

No, it isn't a stupid question. Arrow is used for streaming data between connect server and clients: (https://spark.apache.org/docs/latest/spark-connect-overview.html#how-spark-connect-works)

dengziming · 2025-07-15T11:16:20Z

sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/ClientE2ETestSuite.scala

    checkAnswer(df, (0 until 6).map(i => Row(i)))
  }
+
+  test("SPARK-52770: Support Time type in connect") {


The test seems good, but the test name seems a bit overkill; "Time type" is OK enough.

dengziming

The test name nitpick, it's OK to leave it as it is.

peter-toth · 2025-07-15T13:09:13Z

Merged to master (4.1.0).

Thanks @dongjoon-hyun, @HyukjinKwon, @MaxGekk and @dengziming for the review!

dongjoon-hyun · 2025-07-15T15:04:15Z

Thank you, @peter-toth and all!

…th `4.1.0-preview2` ### What changes were proposed in this pull request? This PR aims to update Spark Connect-generated Swift source code with Apache Spark `4.1.0-preview2`. ### Why are the changes needed? There are many changes from Apache Spark 4.1.0. - apache/spark#52342 - apache/spark#52256 - apache/spark#52271 - apache/spark#52242 - apache/spark#51473 - apache/spark#51653 - apache/spark#52072 - apache/spark#51561 - apache/spark#51563 - apache/spark#51489 - apache/spark#51507 - apache/spark#51462 - apache/spark#51464 - apache/spark#51442 To use the latest bug fixes and new messages to develop for new features of `4.1.0-preview2`. ``` $ git clone -b v4.1.0-preview2 https://github.com/apache/spark.git $ cd spark/sql/connect/common/src/main/protobuf/ $ protoc --swift_out=. spark/connect/*.proto $ protoc --grpc-swift_out=. spark/connect/*.proto // Remove empty GRPC files $ cd spark/connect $ grep 'This file contained no services' * catalog.grpc.swift:// This file contained no services. commands.grpc.swift:// This file contained no services. common.grpc.swift:// This file contained no services. example_plugins.grpc.swift:// This file contained no services. expressions.grpc.swift:// This file contained no services. ml_common.grpc.swift:// This file contained no services. ml.grpc.swift:// This file contained no services. pipelines.grpc.swift:// This file contained no services. relations.grpc.swift:// This file contained no services. types.grpc.swift:// This file contained no services. $ rm catalog.grpc.swift commands.grpc.swift common.grpc.swift example_plugins.grpc.swift expressions.grpc.swift ml_common.grpc.swift ml.grpc.swift pipelines.grpc.swift relations.grpc.swift types.grpc.swift ``` ### Does this PR introduce _any_ user-facing change? Pass the CIs. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #250 from dongjoon-hyun/SPARK-53777. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

github-actions bot added SQL CONNECT labels Jul 13, 2025

peter-toth force-pushed the SPARK-52770-suppot-time-in-connect-proto branch from 634b4ce to 68a1563 Compare July 13, 2025 12:32

github-actions bot added the PYTHON label Jul 13, 2025

MaxGekk mentioned this pull request Jul 13, 2025

[SPARK-52779][SQL][CONNECT] Support TimeType literal in Connect #51462

Closed

dengziming reviewed Jul 13, 2025

View reviewed changes

dongjoon-hyun reviewed Jul 13, 2025

View reviewed changes

peter-toth force-pushed the SPARK-52770-suppot-time-in-connect-proto branch from 68a1563 to 66361ef Compare July 14, 2025 12:11

[SPARK-52770][CONNECT] Support TIME type in connect

b16793b

peter-toth force-pushed the SPARK-52770-suppot-time-in-connect-proto branch from 66361ef to b16793b Compare July 14, 2025 13:18

dongjoon-hyun reviewed Jul 14, 2025

View reviewed changes

dongjoon-hyun approved these changes Jul 14, 2025

View reviewed changes

HyukjinKwon approved these changes Jul 14, 2025

View reviewed changes

dengziming reviewed Jul 15, 2025

View reviewed changes

MaxGekk approved these changes Jul 15, 2025

View reviewed changes

add e2e test

18830e7

dengziming reviewed Jul 15, 2025

View reviewed changes

dengziming approved these changes Jul 15, 2025

View reviewed changes

rename test

98806b2

peter-toth closed this in c84629e Jul 15, 2025

dongjoon-hyun mentioned this pull request Oct 1, 2025

[SPARK-53777] Update Spark Connect-generated Swift source code with 4.1.0-preview2 apache/spark-connect-swift#250

Closed

[SPARK-52770][CONNECT] Support TIME type in connect proto #51464

[SPARK-52770][CONNECT] Support TIME type in connect proto #51464

Uh oh!

Conversation

peter-toth commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

MaxGekk commented Jul 13, 2025

Uh oh!

dengziming left a comment

Choose a reason for hiding this comment

Uh oh!

dengziming Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun 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!

dengziming commented Jul 14, 2025

Uh oh!

peter-toth commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

peter-toth commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun Jul 14, 2025 • 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!

peter-toth Jul 14, 2025 • 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!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

dengziming Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

peter-toth Jul 15, 2025 • 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!

dengziming Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

dengziming left a comment

Choose a reason for hiding this comment

Uh oh!

peter-toth commented Jul 15, 2025

Uh oh!

dongjoon-hyun commented Jul 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[SPARK-52770][CONNECT] Support `TIME` type in connect proto #51464

[SPARK-52770][CONNECT] Support `TIME` type in connect proto #51464

peter-toth commented Jul 13, 2025 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading

peter-toth commented Jul 14, 2025 •

edited

Loading

peter-toth commented Jul 14, 2025 •

edited

Loading

dongjoon-hyun Jul 14, 2025 •

edited

Loading

peter-toth Jul 14, 2025 •

edited

Loading

peter-toth Jul 15, 2025 •

edited

Loading