[SPARK-28015][SQL] Check stringToDate() consumes entire input for the yyyy and yyyy-[m]m formats #25097

MaxGekk · 2019-07-10T12:20:45Z

What changes were proposed in this pull request?

Fix stringToDate() for the formats yyyy and yyyy-[m]m that assumes there are no additional chars after the last components yyyy and [m]m. In the PR, I propose to check that entire input was consumed for the formats.

After the fix, the input 1999 08 01 will be invalid because it matches to the pattern yyyy but the strings contains additional chars 08 01.

Since Spark 1.6.3 ~ 2.4.3, the behavior is the same.

spark-sql> SELECT CAST('1999 08 01' AS DATE);
1999-01-01

This PR makes it return NULL like Hive.

spark-sql> SELECT CAST('1999 08 01' AS DATE);
NULL

How was this patch tested?

Added new checks to DateTimeUtilsSuite for the 1999 08 01 and 1999 08 inputs.

SparkQA · 2019-07-10T14:29:50Z

Test build #107456 has finished for PR 25097 at commit bbe4dcd.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

MaxGekk · 2019-07-10T15:33:59Z

It fails on tests that expect wrong results:

SELECT date '1999 Jan 08'
-- !query 28 schema
struct<DATE '1999-01-01':date>
-- !query 28 output
1999-01-01

I will regenerate expected results.

MaxGekk · 2019-07-10T15:58:08Z

sql/core/src/test/resources/sql-tests/results/pgSQL/date.sql.out

-struct<DATE '1999-01-01':date>
+struct<>
 -- !query 21 output
-1999-01-01


The result is wrong, it is better to throw the exception because the format with spaces (yyyy [m]m [d]d) is not supported currently.

Oops. I missed this. Thank you for the catching this.
cc @wangyum , too.

SparkQA · 2019-07-10T17:00:26Z

Test build #4819 has finished for PR 25097 at commit bbe4dcd.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-07-10T17:17:51Z

sql/core/src/test/resources/sql-tests/results/pgSQL/date.sql.out

+Cannot parse the DATE value: 1999 Jan 08(line 1, pos 7)
+
+== SQL ==
+SELECT date '1999 Jan 08'


Note that this is only for date prefix. With this PR, we will return NULL like Hive.

spark-sql> SELECT CAST('1999 08 01' AS DATE); NULL

@MaxGekk . Could you mention these two cases together in the PR title?

I updated the PR description and JIRA.

SparkQA · 2019-07-10T17:39:48Z

Test build #107470 has finished for PR 25097 at commit 2ee26df.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-07-10T17:42:40Z

Retest this please.

dongjoon-hyun · 2019-07-10T21:09:11Z

Hi, @gatorsmile and @cloud-fan .
This is a correctness issue. We must have this in master.

I'd like to have this in old branches, but this fix is able to cause big behavior changes because sometime date is used for directory partition. How do you think about the backporting?

SparkQA · 2019-07-10T21:15:09Z

Test build #107473 has finished for PR 25097 at commit 2ee26df.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-07-11T01:09:49Z

Merged to master. For the backporting, we can discuss soon.

cloud-fan · 2019-07-11T01:25:03Z

A late LGTM

dongjoon-hyun · 2019-07-11T02:07:06Z

Thank you, @cloud-fan . BTW, how do you think about backporting to branch-2.4?

cloud-fan · 2019-07-11T02:24:25Z

yea let's backport

dongjoon-hyun · 2019-07-11T02:49:25Z

Thanks!

… yyyy and yyyy-[m]m formats Fix `stringToDate()` for the formats `yyyy` and `yyyy-[m]m` that assumes there are no additional chars after the last components `yyyy` and `[m]m`. In the PR, I propose to check that entire input was consumed for the formats. After the fix, the input `1999 08 01` will be invalid because it matches to the pattern `yyyy` but the strings contains additional chars ` 08 01`. Since Spark 1.6.3 ~ 2.4.3, the behavior is the same. ``` spark-sql> SELECT CAST('1999 08 01' AS DATE); 1999-01-01 ``` This PR makes it return NULL like Hive. ``` spark-sql> SELECT CAST('1999 08 01' AS DATE); NULL ``` Added new checks to `DateTimeUtilsSuite` for the `1999 08 01` and `1999 08` inputs. Closes #25097 from MaxGekk/spark-28015-invalid-date-format. Authored-by: Maxim Gekk <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

dongjoon-hyun · 2019-07-11T02:56:03Z

This is cherry-picked to branch-2.4 and DateTimeUtilsSuite is tested locally.

dongjoon-hyun · 2019-07-11T02:56:42Z

I'll test and backport to branch-2.3, too.

… yyyy and yyyy-[m]m formats Fix `stringToDate()` for the formats `yyyy` and `yyyy-[m]m` that assumes there are no additional chars after the last components `yyyy` and `[m]m`. In the PR, I propose to check that entire input was consumed for the formats. After the fix, the input `1999 08 01` will be invalid because it matches to the pattern `yyyy` but the strings contains additional chars ` 08 01`. Since Spark 1.6.3 ~ 2.4.3, the behavior is the same. ``` spark-sql> SELECT CAST('1999 08 01' AS DATE); 1999-01-01 ``` This PR makes it return NULL like Hive. ``` spark-sql> SELECT CAST('1999 08 01' AS DATE); NULL ``` Added new checks to `DateTimeUtilsSuite` for the `1999 08 01` and `1999 08` inputs. Closes #25097 from MaxGekk/spark-28015-invalid-date-format. Authored-by: Maxim Gekk <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 17974e2) Signed-off-by: Dongjoon Hyun <[email protected]>

dongjoon-hyun · 2019-07-11T03:02:00Z

This is cherry-picked and tested in branch-2.3, too.

MaxGekk · 2019-07-11T08:48:52Z

@dongjoon-hyun @srowen @cloud-fan Thank you for your review.

… yyyy and yyyy-[m]m formats Fix `stringToDate()` for the formats `yyyy` and `yyyy-[m]m` that assumes there are no additional chars after the last components `yyyy` and `[m]m`. In the PR, I propose to check that entire input was consumed for the formats. After the fix, the input `1999 08 01` will be invalid because it matches to the pattern `yyyy` but the strings contains additional chars ` 08 01`. Since Spark 1.6.3 ~ 2.4.3, the behavior is the same. ``` spark-sql> SELECT CAST('1999 08 01' AS DATE); 1999-01-01 ``` This PR makes it return NULL like Hive. ``` spark-sql> SELECT CAST('1999 08 01' AS DATE); NULL ``` Added new checks to `DateTimeUtilsSuite` for the `1999 08 01` and `1999 08` inputs. Closes apache#25097 from MaxGekk/spark-28015-invalid-date-format. Authored-by: Maxim Gekk <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

MaxGekk added 2 commits July 10, 2019 14:03

Fix for stringToDate

3619e6a

Check stringToTimestamp too

bbe4dcd

srowen approved these changes Jul 10, 2019

View reviewed changes

MaxGekk added 2 commits July 10, 2019 17:39

Add a test for yyyy-mm

a90c26a

Re-generate expected results for date.sql

2ee26df

MaxGekk commented Jul 10, 2019

View reviewed changes

dongjoon-hyun added the SQL label Jul 10, 2019

dongjoon-hyun approved these changes Jul 10, 2019

View reviewed changes

dongjoon-hyun reviewed Jul 10, 2019

View reviewed changes

dongjoon-hyun closed this in 6532153 Jul 11, 2019

MaxGekk deleted the spark-28015-invalid-date-format branch September 18, 2019 15:58

[SPARK-28015][SQL] Check stringToDate() consumes entire input for the yyyy and yyyy-[m]m formats #25097

[SPARK-28015][SQL] Check stringToDate() consumes entire input for the yyyy and yyyy-[m]m formats #25097

Uh oh!

Conversation

MaxGekk commented Jul 10, 2019 • edited by dongjoon-hyun Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

MaxGekk commented Jul 10, 2019

Uh oh!

MaxGekk Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

dongjoon-hyun Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jul 10, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

dongjoon-hyun commented Jul 10, 2019

Uh oh!

dongjoon-hyun commented Jul 10, 2019

Uh oh!

SparkQA commented Jul 10, 2019

Uh oh!

dongjoon-hyun commented Jul 11, 2019

Uh oh!

cloud-fan commented Jul 11, 2019

Uh oh!

dongjoon-hyun commented Jul 11, 2019

Uh oh!

cloud-fan commented Jul 11, 2019

Uh oh!

dongjoon-hyun commented Jul 11, 2019

Uh oh!

dongjoon-hyun commented Jul 11, 2019

Uh oh!

dongjoon-hyun commented Jul 11, 2019

Uh oh!

dongjoon-hyun commented Jul 11, 2019

Uh oh!

MaxGekk commented Jul 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MaxGekk commented Jul 10, 2019 •

edited by dongjoon-hyun

Loading