Re: [PR] feat: pass ignore_nulls flag to first and last [datafusion-comet]
andygrove merged PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866 -- 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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
rluvaton commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2988542503 sorry, missed your comment -- 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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
andygrove commented on PR #1866:
URL:
https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2976849328
@rluvaton Here is a test that fails in main and passes with your changes in
this PR. Could you add this to `CometAggregateSuite` as part of this PR?
```scala
test("first/last with ignore null") {
val data = Range(0, 8192).flatMap(n => Seq((n, 1), (n, 2))).toDF("a",
"b")
withTempDir { dir =>
val filename = s"${dir.getAbsolutePath}/first_last_ignore_null.parquet"
data.write.parquet(filename)
withSQLConf(CometConf.COMET_BATCH_SIZE.key -> "100") {
spark.read.parquet(filename).createOrReplaceTempView("t1")
for (expr <- Seq("first", "last")) {
// deterministic query that should return one non-null value per
group
val df = spark.sql(s"SELECT a, $expr(IF(b==1,null,b)) IGNORE NULLS
FROM t1 GROUP BY a ORDER BY a")
checkSparkAnswerAndOperator(df)
}
}
}
}
```
--
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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
andygrove commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2976754152 I will create a PR today to add correctness tests for first/last. We can then rebase this PR and make sure that there are no regressions. -- 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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
parthchandra commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2971722751 @andygrove perhaps we can merge this while we wait for the tests to be made more accurate? -- 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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
andygrove commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2970625122 > All the tests for first and last are disabled currently: > > * [Re-enable tests for FIRST/LAST #1646](https://github.com/apache/datafusion-comet/issues/1646) Yes, we should figure out a test approach for these non-deterministic functions before we start making changes to the implementation, -- 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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
rluvaton commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2961843908 All the tests are disabled currently -- 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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
codecov-commenter commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2956336470 ## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/1866?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report Attention: Patch coverage is `0%` with `2 lines` in your changes missing coverage. Please review. > Project coverage is 33.44%. 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 [(`692332f`)](https://app.codecov.io/gh/apache/datafusion-comet/commit/692332fbc6baa2ee90fe3b790e7dff00a7b34ba2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). > Report is 248 commits behind head on main. | [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-comet/pull/1866?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines | |---|---|---| | [...main/scala/org/apache/comet/serde/aggregates.scala](https://app.codecov.io/gh/apache/datafusion-comet/pull/1866?src=pr&el=tree&filepath=spark%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fcomet%2Fserde%2Faggregates.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9jb21ldC9zZXJkZS9hZ2dyZWdhdGVzLnNjYWxh) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-comet/pull/1866?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Additional details and impacted files ```diff @@ Coverage Diff @@ ## main#1866 +/- ## = - Coverage 56.12% 33.44% -22.69% + Complexity 976 800 -176 = Files 119 130 +11 Lines 1174312655 +912 Branches 2251 2357 +106 = - Hits 6591 4232 -2359 - Misses 4012 7456 +3444 + Partials 1140 967 -173 ``` [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/1866?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] feat: pass ignore_nulls flag to first and last [datafusion-comet]
parthchandra commented on PR #1866: URL: https://github.com/apache/datafusion-comet/pull/1866#issuecomment-2956276128 The linked PR did not add a test. Would you be able to? -- 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]
