[multistage] support the rest of the agg functions currently implementable #11208
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #11208 +/- ##
==========================================
- Coverage 0.11% 0.00% -0.12%
==========================================
Files 2229 2213 -16
Lines 119803 119390 -413
Branches 18126 18071 -55
==========================================
- Hits 137 0 -137
+ Misses 119646 119390 -256
+ Partials 20 0 -20
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 16 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
walterddr
commented
Jul 28, 2023
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
Show resolved
Hide resolved
walterddr
commented
Jul 28, 2023
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
Show resolved
Hide resolved
walterddr
commented
Jul 28, 2023
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/AggregationFunctionType.java
Outdated
Show resolved
Hide resolved
xiangfu0
reviewed
Aug 1, 2023
| @@ -46,13 +46,13 @@ public static String literalMapToHintString(Map<Pair<Integer, Integer>, RexExpre | |||
| e.getValue().getDataType().name(), e.getValue().getValue())); | |||
| } | |||
| // semi-colon is used to separate between encoded literals | |||
| return "{" + StringUtils.join(literalStrings, ";") + "}"; | |||
| return "{" + StringUtils.join(literalStrings, ";:;") + "}"; | |||
There was a problem hiding this comment.
the problem is b/c in some agg literals it allows stringify parameter type with ; as the separator. this conflicts with my usage of ; which indicates the "next literal hint".
Thus i used another 3-character splitter ;:; to indicate hint-literal boundaries.
xiangfu0
approved these changes
Aug 1, 2023
walterddr
force-pushed
the
pr_add_remaining_agg
branch
from
8d77bd6
to
e04fc30
Compare
s0nskar
pushed a commit
to s0nskar/pinot
that referenced
this pull request
Aug 10, 2023
…he#11208) * support the rest of the agg functions * support others trivial ones such as DISTINCT_SUM/AVG, MIN_MAX_RANGE, etc * add DISTINCT_COUNT variances * add PERCENTILE_* variances * added TODO comments for the rest of the functions --------- Co-authored-by: Rong Rong <rongr@startree.ai>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add all supported agg functions to follow up with #11034
the following can be added very soon (some complication due to testing)
distinctCountBitmapis not yet supported, can be added later*TupleSketchfunctions not yet supportedSTUnionagg function*MVvariance of agg functions will be added in [multistage] Support multi-value aggregation functions #11216the following functions require some revisit
histogramnot yet supported due to complex ARRAY constructor_version of the stats function such asCOVAR_POPis not supported, (where ascoVarPopis) due to calctie built-in operator table conflict.argmin/argmaxis not supported due to conflicting signature (e.g. arg column at 1st vs arg column at last convention differs between pinot and calcite)funnelCountdue to its complex syntax, there should be an easier solution in v2