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 2020/03/11 01:18:27 UTC
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #5137: Fix
the ConstantValueDictionary
Jackie-Jiang opened a new pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137
1. Implement the insertionIndexOf() for all constant value dictionaries
2. Enhance the integration test to cover that
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request
#5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390983968
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/ConstantValueDoubleDictionary.java
##########
@@ -19,20 +19,25 @@
package org.apache.pinot.core.segment.index.readers;
/**
- * Dictionary for constant-value double
+ * Dictionary of a single double value.
Review comment:
I like the one class approach, and had suggested that earlier to Haibo. Let us not worry too much about a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5137: Fix the default value provider classes
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5137: Fix the default value provider classes
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r391263894
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/ConstantValueDoubleDictionary.java
##########
@@ -19,20 +19,25 @@
package org.apache.pinot.core.segment.index.readers;
/**
- * Dictionary for constant-value double
+ * Dictionary of a single double value.
*/
-
public class ConstantValueDoubleDictionary extends BaseImmutableDictionary {
- final Double _value;
+ private final double _value;
- public ConstantValueDoubleDictionary(Double value) {
+ public ConstantValueDoubleDictionary(double value) {
super(1);
_value = value;
}
@Override
public int insertionIndexOf(String stringValue) {
-
+ double doubleValue = Double.parseDouble(stringValue);
+ if (doubleValue < _value) {
+ return -1;
+ }
+ if (doubleValue > _value) {
+ return -2;
Review comment:
Discussed offline, and that is the general binary search convention.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request
#5137: Fix the default value provider classes
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5137: Fix the default value provider classes
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r391256284
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/ConstantValueDoubleDictionary.java
##########
@@ -19,20 +19,25 @@
package org.apache.pinot.core.segment.index.readers;
/**
- * Dictionary for constant-value double
+ * Dictionary of a single double value.
*/
-
public class ConstantValueDoubleDictionary extends BaseImmutableDictionary {
- final Double _value;
+ private final double _value;
- public ConstantValueDoubleDictionary(Double value) {
+ public ConstantValueDoubleDictionary(double value) {
super(1);
_value = value;
}
@Override
public int insertionIndexOf(String stringValue) {
-
+ double doubleValue = Double.parseDouble(stringValue);
+ if (doubleValue < _value) {
+ return -1;
+ }
+ if (doubleValue > _value) {
+ return -2;
Review comment:
I didnt knoiw we had this convention. The jacvadocs on insertionIndexOf() does not state this in comments. Also, it points to a binarysearch implementation that does not exist (or, I could not locate it).. Can you fix the comments to reflect this convention? thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5137: Fix the
default value provider classes
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #5137: Fix the default value provider classes
URL: https://github.com/apache/incubator-pinot/pull/5137
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request
#5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390748409
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/column/BaseVirtualColumnProvider.java
##########
@@ -39,7 +39,7 @@
public DataFileReader buildReader(VirtualColumnContext context) {
if (context.getFieldSpec().isSingleValueField()) {
- return new ConstantSingleValueInvertedIndex(0);
+ return new ConstantSingleValueSortedIndex(0);
Review comment:
Why is this class renamed to SortedIndex instead of InvertedIndex?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390759640
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/column/BaseVirtualColumnProvider.java
##########
@@ -39,7 +39,7 @@
public DataFileReader buildReader(VirtualColumnContext context) {
if (context.getFieldSpec().isSingleValueField()) {
- return new ConstantSingleValueInvertedIndex(0);
+ return new ConstantSingleValueSortedIndex(0);
Review comment:
Because it is sorted index (sorted index can be used as both forward and inverted index).
The multi-value one should be split into forward and inverted index, but that is out of the scope of this pr, so will address later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] haibow commented on a change in pull request
#5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
haibow commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390728351
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/io/reader/impl/ConstantSingleValueSortedIndex.java
##########
@@ -59,26 +42,32 @@ public int getInt(int row) {
}
@Override
- public int getInt(int rowId, SortedIndexReaderImpl.Context context) {
+ public int getInt(int rowId, ReaderContext context) {
return 0;
}
@Override
- public void close()
- throws IOException {
+ public void readValues(int[] rows, int rowStartPos, int rowSize, int[] values, int valuesStartPos) {
+ Arrays.fill(values, valuesStartPos, valuesStartPos + rowSize, 0);
Review comment:
Good catch. Could you fix another older misuse too:
https://github.com/apache/incubator-pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/io/reader/impl/IntSingleValueDataFileReader.java#L54
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390738442
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/io/reader/impl/ConstantSingleValueSortedIndex.java
##########
@@ -59,26 +42,32 @@ public int getInt(int row) {
}
@Override
- public int getInt(int rowId, SortedIndexReaderImpl.Context context) {
+ public int getInt(int rowId, ReaderContext context) {
return 0;
}
@Override
- public void close()
- throws IOException {
+ public void readValues(int[] rows, int rowStartPos, int rowSize, int[] values, int valuesStartPos) {
+ Arrays.fill(values, valuesStartPos, valuesStartPos + rowSize, 0);
Review comment:
Removed 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5137: Fix
the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#issuecomment-597413763
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=h1) Report
> Merging [#5137](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/330f37f4024c5c7ab20f7fdd729bb029f511ad4f&el=desc) will **increase** coverage by `0.00%`.
> The diff coverage is `10.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5137 +/- ##
=========================================
Coverage 56.56% 56.56%
Complexity 12 12
=========================================
Files 1051 1050 -1
Lines 54125 54142 +17
Branches 8070 8083 +13
=========================================
+ Hits 30614 30626 +12
- Misses 21073 21075 +2
- Partials 2438 2441 +3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...t/index/readers/ConstantValueDoubleDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZURvdWJsZURpY3Rpb25hcnkuamF2YQ==) | `26.66% <0.00%> (-13.34%)` | `0.00 <0.00> (ø)` | |
| [...nt/index/readers/ConstantValueFloatDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUZsb2F0RGljdGlvbmFyeS5qYXZh) | `26.66% <0.00%> (-13.34%)` | `0.00 <0.00> (ø)` | |
| [...t/index/readers/ConstantValueStringDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZVN0cmluZ0RpY3Rpb25hcnkuamF2YQ==) | `31.25% <0.00%> (-18.75%)` | `0.00 <0.00> (ø)` | |
| [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...ment/index/readers/ConstantValueIntDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUludERpY3Rpb25hcnkuamF2YQ==) | `26.66% <11.11%> (-13.34%)` | `0.00 <0.00> (ø)` | |
| [...ent/index/readers/ConstantValueLongDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUxvbmdEaWN0aW9uYXJ5LmphdmE=) | `26.66% <11.11%> (-13.34%)` | `0.00 <0.00> (ø)` | |
| [...io/reader/impl/ConstantSingleValueSortedIndex.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pby9yZWFkZXIvaW1wbC9Db25zdGFudFNpbmdsZVZhbHVlU29ydGVkSW5kZXguamF2YQ==) | `63.63% <40.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...egment/index/column/BaseVirtualColumnProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L2NvbHVtbi9CYXNlVmlydHVhbENvbHVtblByb3ZpZGVyLmphdmE=) | `100.00% <100.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `83.82% <0.00%> (-7.36%)` | `0.00% <0.00%> (ø%)` | |
| [.../realtime/impl/ThreadSafeMutableRoaringBitmap.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL1RocmVhZFNhZmVNdXRhYmxlUm9hcmluZ0JpdG1hcC5qYXZh) | `92.85% <0.00%> (-7.15%)` | `0.00% <0.00%> (ø%)` | |
| ... and [11 more](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=footer). Last update [330f37f...839f126](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #5137: Fix the
ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#issuecomment-597413763
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=h1) Report
> Merging [#5137](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/3e451e7b87bb18592eeb0e97fdd2b0b6fdf43908?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `46.66%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5137 +/- ##
============================================
- Coverage 65.49% 65.47% -0.03%
Complexity 12 12
============================================
Files 1051 1051
Lines 54120 54146 +26
Branches 8068 8081 +13
============================================
+ Hits 35447 35453 +6
- Misses 16052 16073 +21
+ Partials 2621 2620 -1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...egment/index/column/BaseVirtualColumnProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L2NvbHVtbi9CYXNlVmlydHVhbENvbHVtblByb3ZpZGVyLmphdmE=) | `100% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...t/index/readers/ConstantValueStringDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZVN0cmluZ0RpY3Rpb25hcnkuamF2YQ==) | `43.75% <16.66%> (-6.25%)` | `0 <0> (ø)` | |
| [...nt/index/readers/ConstantValueFloatDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUZsb2F0RGljdGlvbmFyeS5qYXZh) | `53.33% <37.5%> (+3.33%)` | `0 <0> (ø)` | :arrow_down: |
| [...t/index/readers/ConstantValueDoubleDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZURvdWJsZURpY3Rpb25hcnkuamF2YQ==) | `53.33% <37.5%> (+13.33%)` | `0 <0> (ø)` | :arrow_down: |
| [...ment/index/readers/ConstantValueIntDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUludERpY3Rpb25hcnkuamF2YQ==) | `53.33% <44.44%> (+3.33%)` | `0 <0> (ø)` | :arrow_down: |
| [...ent/index/readers/ConstantValueLongDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQ29uc3RhbnRWYWx1ZUxvbmdEaWN0aW9uYXJ5LmphdmE=) | `53.33% <44.44%> (+3.33%)` | `0 <0> (ø)` | :arrow_down: |
| [...io/reader/impl/ConstantSingleValueSortedIndex.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pby9yZWFkZXIvaW1wbC9Db25zdGFudFNpbmdsZVZhbHVlU29ydGVkSW5kZXguamF2YQ==) | `81.81% <60%> (ø)` | `0 <0> (?)` | |
| [...server/starter/helix/HelixInstanceDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeEluc3RhbmNlRGF0YU1hbmFnZXIuamF2YQ==) | `79.31% <61.53%> (-2.96%)` | `0 <0> (ø)` | |
| [...er/validation/BrokerResourceValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL0Jyb2tlclJlc291cmNlVmFsaWRhdGlvbk1hbmFnZXIuamF2YQ==) | `25% <0%> (-56.25%)` | `0% <0%> (ø)` | |
| [...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=) | `23.68% <0%> (-21.06%)` | `0% <0%> (ø)` | |
| ... and [26 more](https://codecov.io/gh/apache/incubator-pinot/pull/5137/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=footer). Last update [3e451e7...9cca930](https://codecov.io/gh/apache/incubator-pinot/pull/5137?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] haibow commented on a change in pull request
#5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
haibow commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390728956
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/ConstantValueDoubleDictionary.java
##########
@@ -19,20 +19,25 @@
package org.apache.pinot.core.segment.index.readers;
/**
- * Dictionary for constant-value double
+ * Dictionary of a single double value.
Review comment:
How about constant double value? I also used "single double" earlier but was persuaded by Subbu it sounded a little weird
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull
request #5137: Fix the ConstantValueDictionary
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5137: Fix the ConstantValueDictionary
URL: https://github.com/apache/incubator-pinot/pull/5137#discussion_r390739455
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/ConstantValueDoubleDictionary.java
##########
@@ -19,20 +19,25 @@
package org.apache.pinot.core.segment.index.readers;
/**
- * Dictionary for constant-value double
+ * Dictionary of a single double value.
Review comment:
I prefer the current comment because the dictionary only contains a single value, instead of multiple values of the same value. @mcvsubbu Opinion?
Future plan would be wrap all these classes (forward index, inverted index, dictionary for default column) into one single class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org