You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2023/06/05 23:43:39 UTC
[GitHub] [pinot] gortiz opened a new pull request, #10851: Fix #10713 by giving metainfo more priority than config
gortiz opened a new pull request, #10851:
URL: https://github.com/apache/pinot/pull/10851
This PR fixes #10713 by always trusting the segment metainfo instead of actual index config. That is also specified in the IndexReaderFactory javadoc to avoid misunderstandings in the future.
Contrary to #10715, this does not change how the index configuration is read.
--
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 pull request #10851: Fix #10713 by giving metainfo more priority than config
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #10851:
URL: https://github.com/apache/pinot/pull/10851#issuecomment-1579646593
I'm running the tests again because last time presto driver tests failed due to some network problem. Apart form that, it is ready to merge.
--
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 #10851: Fix #10713 by giving metainfo more priority than config
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10851:
URL: https://github.com/apache/pinot/pull/10851#issuecomment-1577722468
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#10851](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (14fba97) into [master](https://app.codecov.io/gh/apache/pinot/commit/6a7a0668cdd47f0ae0c9b5af218dd2d2b6f1994e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (6a7a066) will **decrease** coverage by `56.46%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10851 +/- ##
=============================================
- Coverage 70.20% 13.75% -56.46%
+ Complexity 6585 439 -6146
=============================================
Files 2170 2120 -50
Lines 116688 114336 -2352
Branches 17661 17388 -273
=============================================
- Hits 81919 15723 -66196
- Misses 29052 97315 +68263
+ Partials 5717 1298 -4419
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `13.75% <0.00%> (+0.12%)` | :arrow_up: |
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/10851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ent/index/column/PhysicalColumnIndexContainer.java](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2NvbHVtbi9QaHlzaWNhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
| [...ocal/segment/index/inverted/InvertedIndexType.java](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2ludmVydGVkL0ludmVydGVkSW5kZXhUeXBlLmphdmE=) | `0.00% <0.00%> (-68.75%)` | :arrow_down: |
| [...al/segment/index/nullvalue/NullValueIndexType.java](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L251bGx2YWx1ZS9OdWxsVmFsdWVJbmRleFR5cGUuamF2YQ==) | `0.00% <ø> (-82.61%)` | :arrow_down: |
| [...egment/local/segment/index/text/TextIndexType.java](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3RleHQvVGV4dEluZGV4VHlwZS5qYXZh) | `0.00% <ø> (-75.00%)` | :arrow_down: |
| [...he/pinot/segment/spi/index/IndexReaderFactory.java](https://app.codecov.io/gh/apache/pinot/pull/10851?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L0luZGV4UmVhZGVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
... and [1716 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10851/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] Jackie-Jiang merged pull request #10851: Fix #10713 by giving metainfo more priority than config
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10851:
URL: https://github.com/apache/pinot/pull/10851
--
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 pull request #10851: Fix #10713 by giving metainfo more priority than config
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #10851:
URL: https://github.com/apache/pinot/pull/10851#issuecomment-1577693842
It would be nice to add a test that verifies that the specified logic is fulfilled by the implementation, but I'm not sure about how to do that. Maybe we can create a segment with the index stored and then mock `SegmentDirectory.Reader.hasIndex` to return false?
--
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