You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/03/24 12:14:55 UTC
[GitHub] [pinot] richardstartin opened a new pull request #8397: Datetime transform functions
richardstartin opened a new pull request #8397:
URL: https://github.com/apache/pinot/pull/8397
This PR starts with a small refactoring to allow `TransformFunctionType`s to have aliases, then introduces `TransformFunction`s implemented in terms of `java.time` for the following functions:
* `year`
* `yearOfWeek`
* `yow`
* `month`
* `week`
* `weekOfYear`
* `quarter`
* `dayOfWeek`
* `dow`
* `dayOfYear`
* `doy`
* `dayOfMonth`
* `day`
* `hour`
* `minute`
* `millisecond`
The transform functions are tested against the `@ScalarFunction`s, which provides a free test oracle for migrating the latter away from Joda time in the future.
The motivation for the change is the high cost of using scalar functions in transforms. Here is the allocation rate observed for
```sql
select count(*), year(event_time) as y, month(event_time) as m from githubEvents group by y, m
```
<img width="854" alt="Screenshot 2022-03-24 at 12 11 19" src="https://user-images.githubusercontent.com/16439049/159913668-602fc431-56e7-43ab-8187-c14952d40f21.png">
vs the similar
```sql
select count(*), dateTrunc('year', event_time) as y, dateTrunc('m', event_time) as y from githubEvents group by y, m
```
<img width="1607" alt="Screenshot 2022-03-24 at 12 11 33" src="https://user-images.githubusercontent.com/16439049/159913696-563defb7-aade-4ffe-97f9-62aab1c01dc8.png">
When scalar function is used, 16% of samples were in verifying accessibility of the `Method` - `@ScalarFunction` should be used only as a last resort.
<img width="782" alt="Screenshot 2022-03-24 at 12 12 00" src="https://user-images.githubusercontent.com/16439049/159913825-21ccc0bd-fb2c-41ce-b60b-932e6749af0e.png">
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin merged pull request #8397: Datetime transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin merged pull request #8397:
URL: https://github.com/apache/pinot/pull/8397
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8397: Datetime transform functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8397:
URL: https://github.com/apache/pinot/pull/8397#issuecomment-1077617883
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8397?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8397](https://codecov.io/gh/apache/pinot/pull/8397?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (95369f4) into [master](https://codecov.io/gh/apache/pinot/commit/5e7c0050f684ac15b3d8a249ad596c2aee0911f5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5e7c005) will **increase** coverage by `33.53%`.
> The diff coverage is `95.10%`.
```diff
@@ Coverage Diff @@
## master #8397 +/- ##
=============================================
+ Coverage 30.54% 64.08% +33.53%
- Complexity 0 4275 +4275
=============================================
Files 1642 1610 -32
Lines 86112 84709 -1403
Branches 13000 12853 -147
=============================================
+ Hits 26304 54285 +27981
+ Misses 57420 26499 -30921
- Partials 2388 3925 +1537
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.00% <95.10%> (?)` | |
| unittests2 | `14.13% <0.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8397?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../transform/function/DateTimeTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vRGF0ZVRpbWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh) | `88.60% <88.60%> (ø)` | |
| [...e/pinot/common/function/TransformFunctionType.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25UeXBlLmphdmE=) | `100.00% <100.00%> (+15.47%)` | :arrow_up: |
| [...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=) | `83.59% <100.00%> (+1.77%)` | :arrow_up: |
| [...va/org/apache/pinot/core/routing/RoutingTable.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1JvdXRpbmdUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...apache/pinot/common/helix/ExtraInstanceConfig.java](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaGVsaXgvRXh0cmFJbnN0YW5jZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1353 more](https://codecov.io/gh/apache/pinot/pull/8397/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8397?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8397?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5e7c005...95369f4](https://codecov.io/gh/apache/pinot/pull/8397?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #8397: Datetime transform functions
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #8397:
URL: https://github.com/apache/pinot/pull/8397#issuecomment-1077589112
I've reverted to JODA because I noticed that `DateTime` objects appear to scalarise in loops like this, where java.time results in allocation. I only tested for `year` but the difference is big enough to stay on JODA:
```
Benchmark (_size) Mode Cnt Score Error Units
BenchmarkTimeAPI.convertJavaTime 1024 avgt 5 35.190 ± 1.019 us/op
BenchmarkTimeAPI.convertJavaTime:·gc.alloc.rate 1024 avgt 5 3842.276 ± 112.354 MB/sec
BenchmarkTimeAPI.convertJavaTime:·gc.alloc.rate.norm 1024 avgt 5 212992.015 ± 0.002 B/op
BenchmarkTimeAPI.convertJavaTime:·gc.churn.G1_Eden_Space 1024 avgt 5 3845.330 ± 353.367 MB/sec
BenchmarkTimeAPI.convertJavaTime:·gc.churn.G1_Eden_Space.norm 1024 avgt 5 213156.517 ± 17414.389 B/op
BenchmarkTimeAPI.convertJavaTime:·gc.churn.G1_Old_Gen 1024 avgt 5 0.003 ± 0.009 MB/sec
BenchmarkTimeAPI.convertJavaTime:·gc.churn.G1_Old_Gen.norm 1024 avgt 5 0.190 ± 0.510 B/op
BenchmarkTimeAPI.convertJavaTime:·gc.count 1024 avgt 5 91.000 counts
BenchmarkTimeAPI.convertJavaTime:·gc.time 1024 avgt 5 54.000 ms
BenchmarkTimeAPI.convertJodaTime 1024 avgt 5 6.541 ± 0.302 us/op
BenchmarkTimeAPI.convertJodaTime:·gc.alloc.rate 1024 avgt 5 ≈ 10⁻⁴ MB/sec
BenchmarkTimeAPI.convertJodaTime:·gc.alloc.rate.norm 1024 avgt 5 0.003 ± 0.001 B/op
BenchmarkTimeAPI.convertJodaTime:·gc.count 1024 avgt 5 ≈ 0 counts
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org