You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2023/09/25 23:29:43 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #11681: fixing ValueReaderComparisonTest
xiangfu0 opened a new pull request, #11681:
URL: https://github.com/apache/pinot/pull/11681
Fixing #11108.
The String constructor may compress charArray, which will break the check in the test.
Tested with 20k runs with random data gen.
--
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] xiangfu0 merged pull request #11681: fixing ValueReaderComparisonTest
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #11681:
URL: https://github.com/apache/pinot/pull/11681
--
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 #11681: fixing ValueReaderComparisonTest
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11681:
URL: https://github.com/apache/pinot/pull/11681#issuecomment-1734632361
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11681?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11681](https://app.codecov.io/gh/apache/pinot/pull/11681?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (60ea493) into [master](https://app.codecov.io/gh/apache/pinot/commit/e6655dd9d417b46db032e8079691d9201ca59edb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e6655dd) will **decrease** coverage by `48.66%`.
> Report is 5 commits behind head on master.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #11681 +/- ##
=============================================
- Coverage 63.10% 14.45% -48.66%
+ Complexity 1121 201 -920
=============================================
Files 2343 2343
Lines 125693 125663 -30
Branches 19310 19312 +2
=============================================
- Hits 79315 18159 -61156
- Misses 40728 105968 +65240
+ Partials 5650 1536 -4114
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-17](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-20](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <ø> (-48.49%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <ø> (-48.66%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <ø> (-48.65%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/11681/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `14.45% <ø> (-0.04%)` | :arrow_down: |
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.
[see 1522 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11681/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 commented on a diff in pull request #11681: fixing ValueReaderComparisonTest
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11681:
URL: https://github.com/apache/pinot/pull/11681#discussion_r1336512941
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/readerwriter/ValueReaderComparisonTest.java:
##########
@@ -145,8 +156,12 @@ void testSuperiorPrefixes(ValueReader reader, int numBytesPerValue, String... st
if (!Character.isLowSurrogate(chars[j])) {
chars[j]++;
String string = new String(chars);
- int signum = strings[i].compareTo(string);
- assertUtf8Comparison(reader, i, numBytesPerValue, string, Integer.compare(signum, 0));
+ // we can only compare if the string is the same length as the stored value.
+ // String constructor may compress the char array.
+ if (string.getBytes(StandardCharsets.UTF_8).length == numBytesPerValue) {
Review Comment:
This is not the correct check. The input value size won't match `numBytesPerValue`
--
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