Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Sep 20, 2019

What changes were proposed in this pull request?

In the PR, I propose to change behavior of the date_part() function in handling null field, and make it the same as PostgreSQL has. If field parameter is null, the function should return null of the double type as PostgreSQL does:

# select date_part(null, date '2019-09-20');
 date_part 
-----------
          
(1 row)

# select pg_typeof(date_part(null, date '2019-09-20'));
    pg_typeof     
------------------
 double precision
(1 row)

Why are the changes needed?

The date_part() function was added to maintain feature parity with PostgreSQL but current behavior of the function is different in handling null as field.

Does this PR introduce any user-facing change?

Yes.

Before:

spark-sql> select date_part(null, date'2019-09-20');
Error in query: null; line 1 pos 7

After:

spark-sql> select date_part(null, date'2019-09-20');
NULL

How was this patch tested?

Add new tests to `DateFunctionsSuite for 2 cases:

  • field = null, source = a date literal
  • field = null, source = a date column

})
val fieldEval = field.eval()
if (fieldEval == null) {
Literal(null, DoubleType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DoubleType ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the example in the description. PostgreSQL returns NULL of the double precision type which is equivalent of DOUBLE type in Spark SQL. Which type would you use here?

@SparkQA
Copy link

SparkQA commented Sep 20, 2019

Test build #111056 has finished for PR 25865 at commit b111cf2.

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

@SparkQA
Copy link

SparkQA commented Sep 20, 2019

Test build #111058 has finished for PR 25865 at commit b39adaa.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you, @MaxGekk .
Merged to master.

@dongjoon-hyun
Copy link
Member

Also, thank you for review, @adrian-wang .

@MaxGekk MaxGekk deleted the date_part-null branch October 5, 2019 19:18
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.

4 participants