You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "wankunde (via GitHub)" <gi...@apache.org> on 2023/12/03 15:31:27 UTC

[PR] [SPARK-46069][SQL][FOLLOWUP] Support unwrap timestamp type to date type [spark]

wankunde opened a new pull request, #44134:
URL: https://github.com/apache/spark/pull/44134

   ### What changes were proposed in this pull request?
   
   A followup of https://github.com/apache/spark/pull/43982, make sure the cast expression is date type when unwrap timestamp type to date type?
   
   ### Why are the changes needed?
   
   A followup.
   
   
   ### Does this PR introduce _any_ user-facing change?
   
   No
   
   ### How was this patch tested?
   
   Exists UT
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44134:
URL: https://github.com/apache/spark/pull/44134#issuecomment-1837729449

   Oh, @wankunde . It's a compilation failure.
   
   ```
   [error] /home/runner/work/spark/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:142:21: fruitless type test: a value of type org.apache.spark.sql.catalyst.expressions.Expression cannot also be a org.apache.spark.sql.types.DateType
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type [spark]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #44134:
URL: https://github.com/apache/spark/pull/44134#discussion_r1413388859


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -139,7 +139,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
       Some(unwrapDateToTimestamp(be, fromExp, date, timeZoneId, evalMode))
 
     case be @ BinaryComparison(
-      Cast(fromExp, _, timeZoneId, evalMode), ts @ Literal(value, _))
+      Cast(fromExp: DateType, _, timeZoneId, evalMode), ts @ Literal(value, _))
         if AnyTimestampType.acceptsType(ts.dataType) && value != null =>

Review Comment:
   `fromExp.dataType == DateType`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Support unwrap timestamp type to date type [spark]

Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #44134:
URL: https://github.com/apache/spark/pull/44134#issuecomment-1837517452

   cc @HyukjinKwon @wangyum 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type [spark]

Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on PR #44134:
URL: https://github.com/apache/spark/pull/44134#issuecomment-1840950539

   @wankunde Please fix the PR description.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44134:
URL: https://github.com/apache/spark/pull/44134#issuecomment-1841165815

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type [spark]

Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #44134:
URL: https://github.com/apache/spark/pull/44134#issuecomment-1837699068

   > Hi, @wankunde .
   > 
   > * The follow-up should not reuse the original PR title. Please use a proper PR title for your code itself.
   > 
   > ```
   > - Cast(fromExp, _, timeZoneId, evalMode), ts @ Literal(value, _))
   > + Cast(fromExp: DateType, _, timeZoneId, evalMode), ts @ Literal(value, _))
   > ```
   > 
   > * Could you re-trigger the CIs or provide the CI link?
   
   Thanks for your remind, update the PR title and re-trigger the CI, thanks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #44134: [SPARK-46069][SQL][FOLLOWUP] Make sure the cast expression is date type when unwrap timestamp type to date type
URL: https://github.com/apache/spark/pull/44134


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org