Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-14 Thread via GitHub 


codecov-commenter commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2972716900

   ## 
[Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/1874?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 Report
   All modified and coverable lines are covered by tests :white_check_mark:
   > Project coverage is 38.91%. Comparing base 
[(`f09f8af`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 to head 
[(`ebad436`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/ebad43690ad7e59a6f81f2cb96bf4a4913f331cb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 260 commits behind head on main.
   
   Additional details and impacted files
   
   
   ```diff
   @@  Coverage Diff  @@
   ##   main#1874   +/-   ##
   =
   - Coverage 56.12%   38.91%   -17.22% 
   + Complexity  976  898   -78 
   =
 Files   119  130   +11 
 Lines 1174312759 +1016 
 Branches   2251 2396  +145 
   =
   - Hits   6591 4965 -1626 
   - Misses 4012 6780 +2768 
   + Partials   1140 1014  -126 
   ```
   
   
   
   [:umbrella: View full report in Codecov by 
Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/1874?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   
   :loudspeaker: Have feedback on the report? [Share it 
here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   
:rocket: New features to boost your workflow: 
   
   - :snowflake: [Test 
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, 
report on failures, and find test suite problems.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-14 Thread via GitHub 


trompa commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2972493584

   @mbutrovich  ive locally moved hour/minute/seconds implementation to a macro.
   I could potentially be able to reuse if for other functions i dont see 
implemented: day/month/year ...
   
   what do you think about it?
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-13 Thread via GitHub 


trompa commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2971008063

   val df = spark.sql("select hour('1969-12-31 16:00:00.0') AS folded_hour")
   
   == Physical Plan ==
   *(1) Project [16 AS folded_hour#0]
   +- *(1) Scan OneRowRelation[]
   
   
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-13 Thread via GitHub 


trompa commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2970687941

   I thinn i can actually just select hour(now()), and parse the query plan,
   would that work?
   
   On Fri, Jun 13, 2025, 14:57 Matt Butrovich ***@***.***> wrote:
   
   > *mbutrovich* left a comment (apache/datafusion-comet#1874)
   > 

   >
   > I guess the argument is that if you do a hour() Spark should
   > evaluate that expression at planning time and we'll get the resulting
   > literal in the query. This test evaluates what happens when the expression
   > runs on table data:
   >
   > 
https://github.com/apache/datafusion-comet/blob/1d0550f606e9f44ee80304a25a5adb53c3ecb99e/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala#L455
   >
   > Maybe we could add a test with a filter that contains an hour()
   > expression and make sure that it gets evaluated on the JVM side and doesn't
   > end up in Comet?
   >
   > —
   > Reply to this email directly, view it on GitHub
   > 
,
   > or unsubscribe
   > 

   > .
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-13 Thread via GitHub 


mbutrovich commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2970308730

   I guess the argument is that if you do a `hour()` Spark should 
evaluate that expression at planning time and we'll get the resulting literal 
in the query. This test evaluates what happens when the expression runs on 
table data:
   
https://github.com/apache/datafusion-comet/blob/1d0550f606e9f44ee80304a25a5adb53c3ecb99e/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala#L455
   
   Maybe we could add a test with a filter that contains an `hour()` expression 
and make sure that it gets evaluated on the JVM side and doesn't end up in 
Comet?


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-13 Thread via GitHub 


trompa commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2969626724

   couldnt find anything.
   I am still connecting the dots between native and jvm. If you give me a hint 
of where this is actually invoked I can try add it


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-12 Thread via GitHub 


mbutrovich commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2968155976

   This is looking good so far! I know it's not your change, but it made me 
wonder: do you know if we have a test that exercises this code path? 
`"Hour(scalar) should be fold in Spark JVM side.".to_string(),`


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Re: [PR] Chore: implement hour func as ScalarUDFImpl [datafusion-comet]

2025-06-12 Thread via GitHub 


mbutrovich commented on PR #1874:
URL: 
https://github.com/apache/datafusion-comet/pull/1874#issuecomment-2966636271

   Thanks for the contribution, @trompa! I'll take a look through this later 
today.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]