Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan closed pull request #44261: [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression URL: https://github.com/apache/spark/pull/44261 -- This is an automated message from the Apache Git Service. To respond to the message, please log on

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on PR #44261: URL: https://github.com/apache/spark/pull/44261#issuecomment-1882886413 thanks, merging to master! -- 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

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1444323099 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ComputeCurrentTimeSuite.scala: ## @@ -163,6 +178,16 @@ class ComputeCurrentTimeSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1444322549 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/ComputeCurrentTimeSuite.scala: ## @@ -51,6 +53,19 @@ class ComputeCurrentTimeSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

beliefer commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1438580819 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala: ## @@ -78,31 +81,14 @@ class DateExpressionsSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1438257053 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala: ## @@ -960,9 +946,9 @@ class DateExpressionsSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

dbatomic commented on PR #44261: URL: https://github.com/apache/spark/pull/44261#issuecomment-1872068792 > LGTM except for [#44261 (comment)](https://github.com/apache/spark/pull/44261#discussion_r1437460354) . Can we verify the test coverage for current datetime functions in

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on PR #44261: URL: https://github.com/apache/spark/pull/44261#issuecomment-1871151807 LGTM except for https://github.com/apache/spark/pull/44261#discussion_r1437460354 . Can we verify the test coverage for current datetime functions in `ComputeCurrentTimeSuite`? --

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437461055 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/EliminateSortsSuite.scala: ## @@ -100,10 +104,10 @@ class EliminateSortsSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437461247 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/FoldablePropagationSuite.scala: ## @@ -75,10 +79,10 @@ class FoldablePropagationSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437461055 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/EliminateSortsSuite.scala: ## @@ -100,10 +104,10 @@ class EliminateSortsSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437460354 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala: ## @@ -79,30 +79,22 @@ class DateExpressionsSuite extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437458586 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala: ## @@ -129,27 +129,20 @@ case class CurrentTimeZone() extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437458793 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala: ## @@ -242,25 +233,20 @@ case class Now() extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437456709 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveInlineTables.scala: ## @@ -68,17 +68,17 @@ object ResolveInlineTables extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437452061 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveInlineTables.scala: ## @@ -68,17 +68,17 @@ object ResolveInlineTables extends

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

cloud-fan commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1437451396 ## sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java: ## @@ -144,6 +146,18 @@ public static byte[] aesDecrypt(byte[]

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

dtenedor commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1427197722 ## sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveInlineTables.scala: ## @@ -33,12 +34,14 @@ import

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

dbatomic commented on PR #44261: URL: https://github.com/apache/spark/pull/44261#issuecomment-1852088896 > > I will try to fix this as part of this PR. > > @dbatomic Could you open a separate PR with a bug fix. In this way, we can backport it without your changes in this PR. I

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

MaxGekk commented on PR #44261: URL: https://github.com/apache/spark/pull/44261#issuecomment-1851950693 > I will try to fix this as part of this PR. @dbatomic Could you open a separate PR with a bug fix. Apparently we can backport it without your changes in this PR. -- This is an

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

dbatomic commented on PR #44261: URL: https://github.com/apache/spark/pull/44261#issuecomment-1851896133 After looking at test failures, there seem to be 'minor' correctness bug for time aware expressions that rely on `ReplaceCurrentLike` rule. For following expression: ```sql

Re: [PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

MaxGekk commented on code in PR #44261: URL: https://github.com/apache/spark/pull/44261#discussion_r1420624345 ## sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala: ## @@ -79,30 +79,22 @@ class DateExpressionsSuite extends

[PR] [SPARK-46331][SQL] Removing CodegenFallback from subset of DateTime expressions and version() expression [spark]

dbatomic opened a new pull request, #44261: URL: https://github.com/apache/spark/pull/44261 ### What changes were proposed in this pull request? This PR moves us a bit closer to removing CodegenFallback class and instead of it relying on RuntimeReplaceable with