Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-234]Support the to_date() with format function in Date and String Type #235

Closed
wants to merge 1 commit into from

Conversation

JkSelf
Copy link
Contributor

@JkSelf JkSelf commented Apr 7, 2021

What changes were proposed in this pull request?

Fix #234

How was this patch tested?

DateFunctionsSuite.scala # function_to_date method

@github-actions
Copy link

github-actions bot commented Apr 7, 2021

#234

class ColumnarUnixTimestamp(timeExp: Expression, format: Expression, original: UnixTimestamp)
extends UnixTimestamp(timeExp, format, original.timeZoneId)
with ColumnarExpression
with Logging {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What types does this operator support? Could you add a buildCheck for supported types?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gazell already has ColumnarUnixTimestamp (convert to unix timestamp in sec).

@@ -504,6 +504,7 @@ object ConverterUtils extends Logging {
case d: StringType =>
case d: DateType =>
case d: DecimalType =>
case d: TimestampType =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this pr depends on the previous pr?
#209

@rui-mo
Copy link
Collaborator

rui-mo commented Apr 8, 2021

depends on: oap-project/arrow#10

@@ -439,7 +436,7 @@ class ColumnarCast(
s"${child.dataType} is not supported in castFLOAT8")
}
} else if (dataType == DateType) {
val supported = List(IntegerType, LongType, DateType)
val supported = List(IntegerType, LongType, DateType, TimestampType)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Current code already has this changes.

@@ -450,6 +447,11 @@ class ColumnarCast(
throw new UnsupportedOperationException(
s"${child.dataType} is not supported in castDECIMAL")
}
} else if (datatype == TimestampType) {
val supported = List(LongType)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Current code already has this changes.

@@ -530,9 +532,21 @@ class ColumnarCast(
TreeBuilder.makeFunction("castFLOAT8", Lists.newArrayList(child_node), resultType)
(funcNode, resultType)
} else if (dataType == DateType) {
val funcNode =
val castNode = if (childType.isInstanceOf[ArrowType.Timestamp]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Current code already has this support.

@@ -541,6 +555,15 @@ class ColumnarCast(
TreeBuilder.makeFunction("castDECIMAL", Lists.newArrayList(child_node), dType)
(funcNode, dType)
}
} else if (dataType == TimestampType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Current code already has the support to cast LongType, DateType, StringType to TimeStampType.
It looks the cast should be tackled according to child type in this piece of code.

@PHILO-HE
Copy link
Collaborator

It looks the proposed functionality in this PR has already implemented in current gazelle code. And I also proposed #622 to support to_date expression.

Let's close this PR. Thanks for this work!

@PHILO-HE PHILO-HE closed this Dec 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support the to_date() with format function
3 participants