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/09/15 19:43:11 UTC
[GitHub] [pinot] walterddr opened a new pull request, #9406: [multistage] Initial commit to support h2 testing
walterddr opened a new pull request, #9406:
URL: https://github.com/apache/pinot/pull/9406
- adding H2 connection to load the same dataset
- verify the final result against entire query resultset
--
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] siddharthteotia commented on a diff in pull request #9406: [multistage] Initial commit to support h2 testing
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9406:
URL: https://github.com/apache/pinot/pull/9406#discussion_r972549589
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -129,14 +124,44 @@ private void consumeInputBlocks() {
}
}
- private List<OrderByExpressionContext> toOrderByExpressions(List<RexExpression> collationKeys,
- List<RelFieldCollation.Direction> collationDirections) {
- List<OrderByExpressionContext> orderByExpressionContextList = new ArrayList<>(collationKeys.size());
- for (int i = 0; i < collationKeys.size(); i++) {
- orderByExpressionContextList.add(new OrderByExpressionContext(ExpressionContext.forIdentifier(
- _dataSchema.getColumnName(((RexExpression.InputRef) collationKeys.get(i)).getIndex())),
- !collationDirections.get(i).isDescending()));
+ private static class SortComparator implements Comparator<Object[]> {
+ private final int _size;
+ private final int[] _valueIndices;
+ private final int[] _multipliers;
+ private final boolean[] _useDoubleComparison;
+
+ public SortComparator(List<RexExpression> collationKeys, List<RelFieldCollation.Direction> collationDirections,
+ DataSchema dataSchema, boolean isNullHandlingEnabled) {
+ DataSchema.ColumnDataType[] columnDataTypes = dataSchema.getColumnDataTypes();
+ _size = collationKeys.size();
+ _valueIndices = new int[_size];
+ _multipliers = new int[_size];
+ _useDoubleComparison = new boolean[_size];
+ for (int i = 0; i < _size; i++) {
+ _valueIndices[i] = ((RexExpression.InputRef) collationKeys.get(i)).getIndex();
+ _multipliers[i] = collationDirections.get(i).isDescending() ? 1 : -1;
+ _useDoubleComparison[i] = columnDataTypes[_valueIndices[i]].isNumber();
+ }
+ }
+
+ @Override
+ public int compare(Object[] o1, Object[] o2) {
+ for (int i = 0; i < _size; i++) {
+ int index = _valueIndices[i];
+ Object v1 = o1[index];
+ Object v2 = o2[index];
+ int result;
+ if (_useDoubleComparison[i]) {
Review Comment:
(nit) can avoid this if check by creating comparators upfront in the constructor
--
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] siddharthteotia commented on a diff in pull request #9406: [multistage] Initial commit to support h2 testing
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9406:
URL: https://github.com/apache/pinot/pull/9406#discussion_r972550066
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -129,14 +124,44 @@ private void consumeInputBlocks() {
}
}
- private List<OrderByExpressionContext> toOrderByExpressions(List<RexExpression> collationKeys,
- List<RelFieldCollation.Direction> collationDirections) {
- List<OrderByExpressionContext> orderByExpressionContextList = new ArrayList<>(collationKeys.size());
- for (int i = 0; i < collationKeys.size(); i++) {
- orderByExpressionContextList.add(new OrderByExpressionContext(ExpressionContext.forIdentifier(
- _dataSchema.getColumnName(((RexExpression.InputRef) collationKeys.get(i)).getIndex())),
- !collationDirections.get(i).isDescending()));
+ private static class SortComparator implements Comparator<Object[]> {
+ private final int _size;
+ private final int[] _valueIndices;
+ private final int[] _multipliers;
+ private final boolean[] _useDoubleComparison;
+
+ public SortComparator(List<RexExpression> collationKeys, List<RelFieldCollation.Direction> collationDirections,
+ DataSchema dataSchema, boolean isNullHandlingEnabled) {
+ DataSchema.ColumnDataType[] columnDataTypes = dataSchema.getColumnDataTypes();
+ _size = collationKeys.size();
+ _valueIndices = new int[_size];
+ _multipliers = new int[_size];
+ _useDoubleComparison = new boolean[_size];
+ for (int i = 0; i < _size; i++) {
+ _valueIndices[i] = ((RexExpression.InputRef) collationKeys.get(i)).getIndex();
+ _multipliers[i] = collationDirections.get(i).isDescending() ? 1 : -1;
+ _useDoubleComparison[i] = columnDataTypes[_valueIndices[i]].isNumber();
+ }
+ }
+
+ @Override
+ public int compare(Object[] o1, Object[] o2) {
+ for (int i = 0; i < _size; i++) {
+ int index = _valueIndices[i];
+ Object v1 = o1[index];
+ Object v2 = o2[index];
+ int result;
+ if (_useDoubleComparison[i]) {
Review Comment:
Actually nvm... it may then have to dereference the actual comparator anyway for dynamic dispatch. So probably not worth 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: 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] siddharthteotia merged pull request #9406: [multistage] Initial commit to support h2 testing
Posted by GitBox <gi...@apache.org>.
siddharthteotia merged PR #9406:
URL: https://github.com/apache/pinot/pull/9406
--
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 #9406: [multistage] Initial commit to support h2 testing
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9406:
URL: https://github.com/apache/pinot/pull/9406#issuecomment-1248764919
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9406?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 [#9406](https://codecov.io/gh/apache/pinot/pull/9406?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (925dbeb) into [master](https://codecov.io/gh/apache/pinot/commit/61fc9190bc1c2f15b3775acf892689a30da6df5e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61fc919) will **decrease** coverage by `2.75%`.
> The diff coverage is `86.20%`.
```diff
@@ Coverage Diff @@
## master #9406 +/- ##
============================================
- Coverage 69.77% 67.02% -2.76%
+ Complexity 5092 4895 -197
============================================
Files 1890 1407 -483
Lines 100654 73187 -27467
Branches 15327 11737 -3590
============================================
- Hits 70231 49053 -21178
+ Misses 25445 20570 -4875
+ Partials 4978 3564 -1414
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.02% <86.20%> (-0.01%)` | :arrow_down: |
| unittests2 | `?` | |
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/9406?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ot/common/function/scalar/ComparisonFunctions.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0NvbXBhcmlzb25GdW5jdGlvbnMuamF2YQ==) | `12.50% <0.00%> (-16.08%)` | :arrow_down: |
| [...not/sql/parsers/rewriter/QueryRewriterFactory.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9yZXdyaXRlci9RdWVyeVJld3JpdGVyRmFjdG9yeS5qYXZh) | `91.66% <ø> (ø)` | |
| [...query/runtime/operator/operands/FilterOperand.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9vcGVyYW5kcy9GaWx0ZXJPcGVyYW5kLmphdmE=) | `66.07% <ø> (ø)` | |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `33.33% <14.28%> (-66.67%)` | :arrow_down: |
| [...not/query/planner/logical/RelToStageConverter.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JlbFRvU3RhZ2VDb252ZXJ0ZXIuamF2YQ==) | `76.92% <50.00%> (ø)` | |
| [...che/pinot/query/planner/logical/RexExpression.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JleEV4cHJlc3Npb24uamF2YQ==) | `74.66% <100.00%> (+0.34%)` | :arrow_up: |
| [...inot/query/planner/logical/RexExpressionUtils.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9sb2dpY2FsL1JleEV4cHJlc3Npb25VdGlscy5qYXZh) | `96.42% <100.00%> (+0.42%)` | :arrow_up: |
| [...he/pinot/query/runtime/operator/OperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9PcGVyYXRvclV0aWxzLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...che/pinot/query/runtime/operator/SortOperator.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9Tb3J0T3BlcmF0b3IuamF2YQ==) | `87.50% <100.00%> (+4.48%)` | :arrow_up: |
| [.../pinot/query/runtime/utils/ServerRequestUtils.java](https://codecov.io/gh/apache/pinot/pull/9406/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS91dGlscy9TZXJ2ZXJSZXF1ZXN0VXRpbHMuamF2YQ==) | `82.97% <100.00%> (+1.36%)` | :arrow_up: |
| ... and [729 more](https://codecov.io/gh/apache/pinot/pull/9406/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) | |
: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=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