You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "shounakmk219 (via GitHub)" <gi...@apache.org> on 2023/05/11 10:35:55 UTC
[GitHub] [pinot] shounakmk219 opened a new pull request, #10757: Improvements on the utility to convert table config to updated format
shounakmk219 opened a new pull request, #10757:
URL: https://github.com/apache/pinot/pull/10757
This PR makes few improvements to the utility ([ref](https://github.com/apache/pinot/pull/10623)) which converts the table configs to updated format. The improvements include
1. skip appending default index configs in order to reduce the verbosity
2. skip null value index config in updated format
3. add `encodingType` to field configs where its missing
--
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] gortiz commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191067266
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexTypeTest.java:
##########
@@ -224,27 +224,48 @@ public void newVarLength()
}
@Test
- public void oldToNewConfConversion()
+ public void oldToNewConfConversionWithOnHeap()
throws IOException {
- _tableConfig.getIndexingConfig().setNoDictionaryColumns(
- JsonUtils.stringToObject("[\"dimInt\"]", _stringListTypeRef)
- );
_tableConfig.getIndexingConfig()
.setOnHeapDictionaryColumns(JsonUtils.stringToObject("[\"dimInt\"]", _stringListTypeRef));
+ convertToUpdatedFormat();
+ FieldConfig fieldConfig = getFieldConfigByColumn("dimInt");
+ assertNotNull(fieldConfig.getIndexes().get(new DictionaryIndexType().getPrettyName()));
Review Comment:
Shoudln't we check here that the DictionaryConfig is on heap here?
Also, I think it would be better to do not instantiate a new DIctionaryIndexType and instead use StandardIndexes.dictionary()
--
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] gortiz commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191040472
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/H3IndexResolution.java:
##########
@@ -73,4 +74,21 @@ public List<Integer> getResolutions() {
public int getLowestResolution() {
return Integer.numberOfTrailingZeros(_resolutions);
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ H3IndexResolution that = (H3IndexResolution) o;
Review Comment:
Here you are not calling super.equals, so enable/disable is not being compared
--
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] gortiz commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1192127114
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/H3IndexResolution.java:
##########
@@ -73,4 +74,21 @@ public List<Integer> getResolutions() {
public int getLowestResolution() {
return Integer.numberOfTrailingZeros(_resolutions);
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ H3IndexResolution that = (H3IndexResolution) o;
Review Comment:
You are right
--
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] gortiz commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191030397
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexType.java:
##########
@@ -348,6 +353,50 @@ protected Dictionary createIndexReader(PinotDataBuffer dataBuffer, ColumnMetadat
@Override
protected void handleIndexSpecificCleanup(TableConfig tableConfig) {
IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ List<String> noDictionaryColumns = indexingConfig.getNoDictionaryColumns() == null
+ ? Lists.newArrayList()
+ : indexingConfig.getNoDictionaryColumns();
+ List<FieldConfig> fieldConfigList = tableConfig.getFieldConfigList() == null
+ ? Lists.newArrayList()
+ : tableConfig.getFieldConfigList();
+
+ List<FieldConfig> configsToUpdate = new ArrayList<>();
+ for (FieldConfig fieldConfig : fieldConfigList) {
+ // skip further computation of field configs which already has RAW encodingType
+ if (fieldConfig.getEncodingType() == FieldConfig.EncodingType.RAW) {
+ continue;
+ }
+ // ensure encodingType is RAW on noDictionaryColumns
+ if (noDictionaryColumns.remove(fieldConfig.getName())) {
+ configsToUpdate.add(fieldConfig);
+ }
+ try {
+ DictionaryIndexConfig indexConfig = new ObjectMapper()
+ .treeToValue(fieldConfig.getIndexes().get(getPrettyName()), DictionaryIndexConfig.class);
+ // ensure encodingType is RAW where dictionary index config has disabled = true
+ if (indexConfig.isDisabled()) {
+ configsToUpdate.add(fieldConfig);
+ }
+ } catch (JsonProcessingException | NullPointerException ignore) {
Review Comment:
Why do we ignore the exception here? I would say that we need to throw it. Is there something I'm missing?
--
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] shounakmk219 commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "shounakmk219 (via GitHub)" <gi...@apache.org>.
shounakmk219 commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191064720
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/H3IndexResolution.java:
##########
@@ -73,4 +74,21 @@ public List<Integer> getResolutions() {
public int getLowestResolution() {
return Integer.numberOfTrailingZeros(_resolutions);
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ H3IndexResolution that = (H3IndexResolution) o;
Review Comment:
I guess you are confusing it with `H3IndexConfig` which has a super.equals call. `H3IndexResolution` does not have a parent 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.
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] gortiz commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191022079
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/NullValueIndexTest.java:
##########
@@ -37,13 +35,7 @@ public void oldToNewConfConversion() {
_tableConfig.getIndexingConfig().setNullHandlingEnabled(true);
convertToUpdatedFormat();
assertNotNull(_tableConfig.getFieldConfigList());
Review Comment:
This shouldn't be a requirement of the test. Whether the fieldConfigList is null or not is not relevant for null vector type, given it does nothing.
What I would check is that getIndexingConfig().isNullHandlingEnabled is still true
--
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] shounakmk219 commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "shounakmk219 (via GitHub)" <gi...@apache.org>.
shounakmk219 commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191060536
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexType.java:
##########
@@ -348,6 +353,50 @@ protected Dictionary createIndexReader(PinotDataBuffer dataBuffer, ColumnMetadat
@Override
protected void handleIndexSpecificCleanup(TableConfig tableConfig) {
IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ List<String> noDictionaryColumns = indexingConfig.getNoDictionaryColumns() == null
+ ? Lists.newArrayList()
+ : indexingConfig.getNoDictionaryColumns();
+ List<FieldConfig> fieldConfigList = tableConfig.getFieldConfigList() == null
+ ? Lists.newArrayList()
+ : tableConfig.getFieldConfigList();
+
+ List<FieldConfig> configsToUpdate = new ArrayList<>();
+ for (FieldConfig fieldConfig : fieldConfigList) {
+ // skip further computation of field configs which already has RAW encodingType
+ if (fieldConfig.getEncodingType() == FieldConfig.EncodingType.RAW) {
+ continue;
+ }
+ // ensure encodingType is RAW on noDictionaryColumns
+ if (noDictionaryColumns.remove(fieldConfig.getName())) {
+ configsToUpdate.add(fieldConfig);
+ }
+ try {
+ DictionaryIndexConfig indexConfig = new ObjectMapper()
+ .treeToValue(fieldConfig.getIndexes().get(getPrettyName()), DictionaryIndexConfig.class);
+ // ensure encodingType is RAW where dictionary index config has disabled = true
+ if (indexConfig.isDisabled()) {
+ configsToUpdate.add(fieldConfig);
+ }
+ } catch (JsonProcessingException | NullPointerException ignore) {
Review Comment:
Let me fix this. I was assuming there will never be a JsonProcessingException exception as the index config is code generated and purposefully ignored the NPE when there are no indexes.
--
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] saurabhd336 commented on pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 commented on PR #10757:
URL: https://github.com/apache/pinot/pull/10757#issuecomment-1547575421
LGTM
--
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] saurabhd336 merged pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "saurabhd336 (via GitHub)" <gi...@apache.org>.
saurabhd336 merged PR #10757:
URL: https://github.com/apache/pinot/pull/10757
--
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] gortiz commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191028548
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexType.java:
##########
@@ -348,6 +353,50 @@ protected Dictionary createIndexReader(PinotDataBuffer dataBuffer, ColumnMetadat
@Override
protected void handleIndexSpecificCleanup(TableConfig tableConfig) {
IndexingConfig indexingConfig = tableConfig.getIndexingConfig();
+ List<String> noDictionaryColumns = indexingConfig.getNoDictionaryColumns() == null
+ ? Lists.newArrayList()
+ : indexingConfig.getNoDictionaryColumns();
+ List<FieldConfig> fieldConfigList = tableConfig.getFieldConfigList() == null
+ ? Lists.newArrayList()
+ : tableConfig.getFieldConfigList();
+
+ List<FieldConfig> configsToUpdate = new ArrayList<>();
+ for (FieldConfig fieldConfig : fieldConfigList) {
+ // skip further computation of field configs which already has RAW encodingType
+ if (fieldConfig.getEncodingType() == FieldConfig.EncodingType.RAW) {
+ continue;
+ }
+ // ensure encodingType is RAW on noDictionaryColumns
+ if (noDictionaryColumns.remove(fieldConfig.getName())) {
+ configsToUpdate.add(fieldConfig);
+ }
+ try {
+ DictionaryIndexConfig indexConfig = new ObjectMapper()
Review Comment:
Do not instantiate ObjectMapper here, use the JsonUtils we already have. We do that in order to use the same Jackson config on all the code.
--
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 #10757: Improvements on the utility to convert table config to updated format
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10757:
URL: https://github.com/apache/pinot/pull/10757#issuecomment-1543842395
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10757?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 [#10757](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (826f0af) into [master](https://app.codecov.io/gh/apache/pinot/commit/4f5030530f55e9a08ad9e3cdbf8a5d96319bbb57?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4f50305) will **decrease** coverage by `42.98%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10757 +/- ##
=============================================
- Coverage 70.47% 27.50% -42.98%
+ Complexity 6471 58 -6413
=============================================
Files 2143 2137 -6
Lines 115167 115053 -114
Branches 17352 17341 -11
=============================================
- Hits 81165 31645 -49520
- Misses 28373 80244 +51871
+ Partials 5629 3164 -2465
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.26% <0.00%> (+0.03%)` | :arrow_up: |
| integration2 | `23.86% <0.00%> (-0.07%)` | :arrow_down: |
| unittests1 | `?` | |
| 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://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../segment/index/dictionary/DictionaryIndexType.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2RpY3Rpb25hcnkvRGljdGlvbmFyeUluZGV4VHlwZS5qYXZh) | `0.00% <0.00%> (-91.67%)` | :arrow_down: |
| [...al/segment/index/nullvalue/NullValueIndexType.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L251bGx2YWx1ZS9OdWxsVmFsdWVJbmRleFR5cGUuamF2YQ==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| [...che/pinot/segment/spi/index/AbstractIndexType.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L0Fic3RyYWN0SW5kZXhUeXBlLmphdmE=) | `0.00% <0.00%> (-93.34%)` | :arrow_down: |
| [...ache/pinot/segment/spi/index/RangeIndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L1JhbmdlSW5kZXhDb25maWcuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/segment/spi/index/TextIndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L1RleHRJbmRleENvbmZpZy5qYXZh) | `0.00% <0.00%> (-83.79%)` | :arrow_down: |
| [...pinot/segment/spi/index/creator/H3IndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L2NyZWF0b3IvSDNJbmRleENvbmZpZy5qYXZh) | `0.00% <0.00%> (-82.36%)` | :arrow_down: |
| [...ot/segment/spi/index/reader/H3IndexResolution.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L3JlYWRlci9IM0luZGV4UmVzb2x1dGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/IndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0luZGV4Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...apache/pinot/spi/config/table/JsonIndexConfig.java](https://app.codecov.io/gh/apache/pinot/pull/10757?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0pzb25JbmRleENvbmZpZy5qYXZh) | `0.00% <0.00%> (-91.90%)` | :arrow_down: |
... and [1527 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10757/indirect-changes?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
[GitHub] [pinot] shounakmk219 commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "shounakmk219 (via GitHub)" <gi...@apache.org>.
shounakmk219 commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191055185
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/NullValueIndexTest.java:
##########
@@ -37,13 +35,7 @@ public void oldToNewConfConversion() {
_tableConfig.getIndexingConfig().setNullHandlingEnabled(true);
convertToUpdatedFormat();
assertNotNull(_tableConfig.getFieldConfigList());
Review Comment:
Makes sense, will add the check.
--
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] shounakmk219 commented on a diff in pull request #10757: Improvements on the utility to convert table config to updated format
Posted by "shounakmk219 (via GitHub)" <gi...@apache.org>.
shounakmk219 commented on code in PR #10757:
URL: https://github.com/apache/pinot/pull/10757#discussion_r1191110079
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexTypeTest.java:
##########
@@ -224,27 +224,48 @@ public void newVarLength()
}
@Test
- public void oldToNewConfConversion()
+ public void oldToNewConfConversionWithOnHeap()
throws IOException {
- _tableConfig.getIndexingConfig().setNoDictionaryColumns(
- JsonUtils.stringToObject("[\"dimInt\"]", _stringListTypeRef)
- );
_tableConfig.getIndexingConfig()
.setOnHeapDictionaryColumns(JsonUtils.stringToObject("[\"dimInt\"]", _stringListTypeRef));
+ convertToUpdatedFormat();
+ FieldConfig fieldConfig = getFieldConfigByColumn("dimInt");
+ assertNotNull(fieldConfig.getIndexes().get(new DictionaryIndexType().getPrettyName()));
Review Comment:
Nice catch, I even missed the use var length check below.
--
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