Skip to content

[SPARK-47396][SQL] Add a general mapping for TIME WITHOUT TIME ZONE to TimestampNTZType - #45519

Closed
yaooqinn wants to merge 2 commits into
apache:masterfrom
yaooqinn:SPARK-47396
Closed

yaooqinn wants to merge 2 commits into
apache:masterfrom
yaooqinn:SPARK-47396

Conversation

@yaooqinn

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Add a general mapping for TIME WITHOUT TIME ZONE to TimestampNTZType

Why are the changes needed?

TIME WITHOUT TIME ZONE should be able to be configured to map to TimestampNTZType like TIMESTAMP WITHOUT TIME ZONE

Does this PR introduce any user-facing change?

yes, TIME WITHOUT TIME ZONE can be mapped to TimestampNTZType when preferTimestampNTZ is true

How was this patch tested?

new tests

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

no

@github-actions github-actions Bot added the SQL label Mar 14, 2024

@dongjoon-hyun dongjoon-hyun 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.

+1, LGTM. Thank you for making this general, @yaooqinn .

Pending CIs.

@yaooqinn

Copy link
Copy Markdown
Member Author

Thank you @dongjoon-hyun

@dongjoon-hyun

Copy link
Copy Markdown
Member

Oh, it seems that the newly added test case fails at PostgresIntegrationSuite.

[info] PostgresIntegrationSuite:
[info] - Type mapping for various types (209 milliseconds)
[info] - Basic write test (293 milliseconds)
[info] - Creating a table with shorts and floats (80 milliseconds)
[info] - SPARK-47390: Convert TIMESTAMP/TIME WITH TIME ZONE regardless of preferTimestampNTZ *** FAILED *** (55 milliseconds)
[info]   1970-01-01T09:22:31.949271 was not instance of java.sql.Timestamp (PostgresIntegrationSuite.scala:293)

@dongjoon-hyun

Copy link
Copy Markdown
Member

Oh, it fails at the previous PR (SPARK-47390) instead of this PR, SPARK-47396.

@yaooqinn

Copy link
Copy Markdown
Member Author

Oops...I Will check the pg time types again. The typeNames for times are copied from existing code,and they might be wrong

dongjoon-hyun pushed a commit that referenced this pull request Mar 14, 2024
…esDialect

### What changes were proposed in this pull request?

This PR fixes a bug in SPARK-47390, we shall separate TIME from TIMESTAMP case-match branch

### Why are the changes needed?

bugfix

### Does this PR introduce _any_ user-facing change?

no

### How was this patch tested?

local test with #45519 merged together

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

Closes #45522 from yaooqinn/SPARK-47390-F.

Authored-by: Kent Yao <yao@apache.org>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
@dongjoon-hyun

Copy link
Copy Markdown
Member

#45522 is merged.

Could you rebase this PR to the master branch once more?

@yaooqinn

Copy link
Copy Markdown
Member Author

Thank you very much @dongjoon-hyun

@dongjoon-hyun

Copy link
Copy Markdown
Member

Merged to master for Apache Spark 4.0.0. Thank you, @yaooqinn .

@yaooqinn
yaooqinn deleted the SPARK-47396 branch March 15, 2024 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants