You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "walterddr (via GitHub)" <gi...@apache.org> on 2023/07/23 18:32:48 UTC
[GitHub] [pinot] walterddr opened a new pull request, #11153: add theta sketch scalar
walterddr opened a new pull request, #11153:
URL: https://github.com/apache/pinot/pull/11153
syntax examples:
```
select
GET_SKETCH_ESTIMATE(DISTINCTCOUNTRAWTHETASKETCH(runs, '')),
GET_SKETCH_ESTIMATE(DISTINCTCOUNTRAWTHETASKETCH(runs, '')),
GET_SKETCH_ESTIMATE(SKETCH_DIFF(DISTINCTCOUNTRAWTHETASKETCH(runs, ''), DISTINCTCOUNTRAWTHETASKETCH(runs, ''))),
GET_SKETCH_ESTIMATE(SKETCH_UNION(DISTINCTCOUNTRAWTHETASKETCH(runs, ''), DISTINCTCOUNTRAWTHETASKETCH(runs, ''))),
GET_SKETCH_ESTIMATE(SKETCH_INTERSECT(DISTINCTCOUNTRAWTHETASKETCH(runs, ''), DISTINCTCOUNTRAWTHETASKETCH(runs, '')))
from baseballStats limit 10
```
--
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] cbalci commented on pull request #11153: add theta sketch scalar
Posted by "cbalci (via GitHub)" <gi...@apache.org>.
cbalci commented on PR #11153:
URL: https://github.com/apache/pinot/pull/11153#issuecomment-1648292426
This is pretty cool, but I'd suggest we prefix the function names with sketch types, ie `THETA_SKETCH_GET_ESTIMATE`, since we use different sketches (KLL, Tuples etc.) and more coming.
--
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] cbalci commented on pull request #11153: add theta sketch scalar
Posted by "cbalci (via GitHub)" <gi...@apache.org>.
cbalci commented on PR #11153:
URL: https://github.com/apache/pinot/pull/11153#issuecomment-1648444465
Right, 'sketch' is a generic term for various probabilistic data structures and different sketches have different binary layouts and serializers/deserializers. It would be hard to write a generic function which handles all. The one you are using in this PR is for `ThetaSketch` only.
Even if we manage to identify they sketch type from serialized form, interfaces may differ. For example `ThetaSketch` and `TuplesSketch` are 'distinctcount' type sketches and offer a `.getEstimate` interface, while `KLLSketch` is a 'percentile' type sketch which has a `.getQuantile` interface.
Hope this makes sense.
--
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] walterddr commented on pull request #11153: add theta sketch scalar
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #11153:
URL: https://github.com/apache/pinot/pull/11153#issuecomment-1648457978
ok so to summarize:
- we should have a `getSketchEstimate` and `getSketchQuantile` method which should figure out if the sketch object allows such function (this can be done as a follow up with customized defined type
- for setUnion/Intersect/Diff only the same type of sketches are allowed to perform set operation.
feels like this should be handled by type matching instead of function name.
--
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] walterddr merged pull request #11153: add theta sketch scalar
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #11153:
URL: https://github.com/apache/pinot/pull/11153
--
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] walterddr commented on pull request #11153: add theta sketch scalar
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #11153:
URL: https://github.com/apache/pinot/pull/11153#issuecomment-1648299309
> This is pretty cool, but I'd suggest we prefix the function names with sketch types, ie `THETA_SKETCH_GET_ESTIMATE`, since we use different sketches (KLL, Tuples etc.) and more coming.
Ahh. LOL i didn't know KLL and Tuples are actually a different sketches :-) thank you for the notes.
--
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 #11153: add theta sketch scalar
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11153:
URL: https://github.com/apache/pinot/pull/11153#issuecomment-1648269766
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11153?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11153](https://app.codecov.io/gh/apache/pinot/pull/11153?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (6252a71) into [master](https://app.codecov.io/gh/apache/pinot/commit/fe2b013a657e1ad6ac508a9a37933961bc4c408b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (fe2b013) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11153 +/- ##
==========================================
- Coverage 0.11% 0.11% -0.01%
==========================================
Files 2208 2208
Lines 118473 118616 +143
Branches 17922 17948 +26
==========================================
Hits 137 137
- Misses 118316 118459 +143
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `0.00% <0.00%> (ø)` | |
| integration1temurin17 | `0.00% <0.00%> (ø)` | |
| integration1temurin20 | `0.00% <0.00%> (ø)` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin11 | `?` | |
| unittests1temurin17 | `0.00% <0.00%> (ø)` | |
| unittests1temurin20 | `0.00% <0.00%> (ø)` | |
| unittests2temurin11 | `0.11% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2temurin17 | `?` | |
| unittests2temurin20 | `0.11% <0.00%> (-0.01%)` | :arrow_down: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/11153?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...he/pinot/core/function/scalar/SketchFunctions.java](https://app.codecov.io/gh/apache/pinot/pull/11153?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9mdW5jdGlvbi9zY2FsYXIvU2tldGNoRnVuY3Rpb25zLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/segment/spi/AggregationFunctionType.java](https://app.codecov.io/gh/apache/pinot/pull/11153?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL0FnZ3JlZ2F0aW9uRnVuY3Rpb25UeXBlLmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [14 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11153/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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] mayankshriv commented on pull request #11153: add theta sketch scalar
Posted by "mayankshriv (via GitHub)" <gi...@apache.org>.
mayankshriv commented on PR #11153:
URL: https://github.com/apache/pinot/pull/11153#issuecomment-1646965621
Why not put this in OSS?
--
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