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 2025/10/29 01:24:47 UTC
[PR] Migrate all the usage of com.google.common.collect.ImmutableList to java.util.List [pinot]
xiangfu0 opened a new pull request, #17101:
URL: https://github.com/apache/pinot/pull/17101
Enforce checkstyle failure for com.google.common.collect.ImmutableList
--
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
Re: [PR] Migrate all the usage of com.google.common.collect.ImmutableList to java.util.List [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17101:
URL: https://github.com/apache/pinot/pull/17101#discussion_r2471512860
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchangeTest.java:
##########
@@ -86,9 +86,11 @@ public void shouldSendEosBlockToAllDestinations()
public void shouldSendDataBlocksOnlyToTargetDestination()
throws Exception {
// Given:
- List<SendingMailbox> destinations = ImmutableList.of(_mailbox1);
+ List<SendingMailbox> destinations = List.of(_mailbox1);
BlockExchange exchange = new TestBlockExchange(destinations);
- RowHeapDataBlock block = new RowHeapDataBlock(ImmutableList.of(new Object[]{"val"}),
+ RowHeapDataBlock block = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"val"});
+ }},
Review Comment:
Replace the anonymous ArrayList with double brace initialization with List.of() for better readability and performance. The double brace initialization creates an anonymous subclass and holds an implicit reference to the outer instance.
```suggestion
RowHeapDataBlock block = new RowHeapDataBlock(
List.of(new Object[]{"val"}),
```
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchangeTest.java:
##########
@@ -139,18 +143,22 @@ public void shouldSignalEarlyTerminationProperly()
public void shouldSplitBlocks()
throws Exception {
// Given:
- List<SendingMailbox> destinations = ImmutableList.of(_mailbox1);
+ List<SendingMailbox> destinations = List.of(_mailbox1);
DataSchema schema = new DataSchema(new String[]{"foo"}, new ColumnDataType[]{ColumnDataType.STRING});
RowHeapDataBlock inBlock =
- new RowHeapDataBlock(ImmutableList.of(new Object[]{"one"}, new Object[]{"two"}), schema);
+ new RowHeapDataBlock(List.of(new Object[]{"one"}, new Object[]{"two"}), schema);
- RowHeapDataBlock outBlockOne = new RowHeapDataBlock(ImmutableList.of(new Object[]{"one"}), schema);
+ RowHeapDataBlock outBlockOne = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"one"});
+ }}, schema);
Review Comment:
Replace the anonymous ArrayList with double brace initialization with List.of() for better readability and performance. The double brace initialization creates an anonymous subclass and holds an implicit reference to the outer instance.
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchangeTest.java:
##########
@@ -139,18 +143,22 @@ public void shouldSignalEarlyTerminationProperly()
public void shouldSplitBlocks()
throws Exception {
// Given:
- List<SendingMailbox> destinations = ImmutableList.of(_mailbox1);
+ List<SendingMailbox> destinations = List.of(_mailbox1);
DataSchema schema = new DataSchema(new String[]{"foo"}, new ColumnDataType[]{ColumnDataType.STRING});
RowHeapDataBlock inBlock =
- new RowHeapDataBlock(ImmutableList.of(new Object[]{"one"}, new Object[]{"two"}), schema);
+ new RowHeapDataBlock(List.of(new Object[]{"one"}, new Object[]{"two"}), schema);
- RowHeapDataBlock outBlockOne = new RowHeapDataBlock(ImmutableList.of(new Object[]{"one"}), schema);
+ RowHeapDataBlock outBlockOne = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"one"});
+ }}, schema);
- RowHeapDataBlock outBlockTwo = new RowHeapDataBlock(ImmutableList.of(new Object[]{"two"}), schema);
+ RowHeapDataBlock outBlockTwo = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"two"});
+ }}, schema);
Review Comment:
Replace the anonymous ArrayList with double brace initialization with List.of() for better readability and performance. The double brace initialization creates an anonymous subclass and holds an implicit reference to the outer instance.
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchangeTest.java:
##########
@@ -108,9 +110,11 @@ public void shouldSendDataBlocksOnlyToTargetDestination()
public void shouldSignalEarlyTerminationProperly()
throws Exception {
// Given:
- List<SendingMailbox> destinations = ImmutableList.of(_mailbox1, _mailbox2);
+ List<SendingMailbox> destinations = List.of(_mailbox1, _mailbox2);
BlockExchange exchange = new TestBlockExchange(destinations);
- RowHeapDataBlock block = new RowHeapDataBlock(ImmutableList.of(new Object[]{"val"}),
+ RowHeapDataBlock block = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"val"});
+ }},
Review Comment:
Replace the anonymous ArrayList with double brace initialization with List.of() for better readability and performance. The double brace initialization creates an anonymous subclass and holds an implicit reference to the outer instance.
--
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
Re: [PR] Cleanup: Migrate all the usage of com.google.common.collect.ImmutableList to java.util.List [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17101:
URL: https://github.com/apache/pinot/pull/17101#discussion_r2475597826
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchangeTest.java:
##########
@@ -86,9 +86,11 @@ public void shouldSendEosBlockToAllDestinations()
public void shouldSendDataBlocksOnlyToTargetDestination()
throws Exception {
// Given:
- List<SendingMailbox> destinations = ImmutableList.of(_mailbox1);
+ List<SendingMailbox> destinations = List.of(_mailbox1);
BlockExchange exchange = new TestBlockExchange(destinations);
- RowHeapDataBlock block = new RowHeapDataBlock(ImmutableList.of(new Object[]{"val"}),
+ RowHeapDataBlock block = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"val"});
+ }},
Review Comment:
works
--
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
Re: [PR] Cleanup: Migrate all the usage of com.google.common.collect.ImmutableList to java.util.List [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #17101:
URL: https://github.com/apache/pinot/pull/17101
--
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
Re: [PR] Cleanup: Migrate all the usage of com.google.common.collect.ImmutableList to java.util.List [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on code in PR #17101:
URL: https://github.com/apache/pinot/pull/17101#discussion_r2475287518
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/exchange/BlockExchangeTest.java:
##########
@@ -86,9 +86,11 @@ public void shouldSendEosBlockToAllDestinations()
public void shouldSendDataBlocksOnlyToTargetDestination()
throws Exception {
// Given:
- List<SendingMailbox> destinations = ImmutableList.of(_mailbox1);
+ List<SendingMailbox> destinations = List.of(_mailbox1);
BlockExchange exchange = new TestBlockExchange(destinations);
- RowHeapDataBlock block = new RowHeapDataBlock(ImmutableList.of(new Object[]{"val"}),
+ RowHeapDataBlock block = new RowHeapDataBlock(new ArrayList<>() {{
+ add(new Object[]{"val"});
+ }},
Review Comment:
Can we use `Collections.singletonList` instead in all these places?
--
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
Re: [PR] Migrate all the usage of com.google.common.collect.ImmutableList to java.util.List [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17101:
URL: https://github.com/apache/pinot/pull/17101#issuecomment-3459383405
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17101?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `84.31373%` with `8 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.20%. Comparing base ([`bffd1e9`](https://app.codecov.io/gh/apache/pinot/commit/bffd1e9692d5c893a40339cf52ab57f375bd298b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e648e2a`](https://app.codecov.io/gh/apache/pinot/commit/e648e2a8db89862da702d6042e582192b086533f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/17101?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ache/pinot/calcite/rel/traits/TraitAssignment.java](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&filepath=pinot-query-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcalcite%2Frel%2Ftraits%2FTraitAssignment.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2FsY2l0ZS9yZWwvdHJhaXRzL1RyYWl0QXNzaWdubWVudC5qYXZh) | 57.14% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...not/segment/local/customobject/QuantileDigest.java](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fcustomobject%2FQuantileDigest.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvUXVhbnRpbGVEaWdlc3QuamF2YQ==) | 60.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../pinot/segment/local/segment/creator/Fixtures.java](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Fcreator%2FFixtures.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvRml4dHVyZXMuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/pinot/tsdb/planner/TimeSeriesPlanFragmenter.java](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&filepath=pinot-timeseries%2Fpinot-timeseries-planner%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Ftsdb%2Fplanner%2FTimeSeriesPlanFragmenter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtdGltZXNlcmllcy9waW5vdC10aW1lc2VyaWVzLXBsYW5uZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3RzZGIvcGxhbm5lci9UaW1lU2VyaWVzUGxhbkZyYWdtZW50ZXIuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17101?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (bffd1e9) and HEAD (e648e2a). Click for more details.
>
> <details><summary>HEAD has 32 uploads less than BASE</summary>
>
>| Flag | BASE (bffd1e9) | HEAD (e648e2a) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|3|1|
>|temurin|9|1|
>|java-11|4|0|
>|unittests2|1|0|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17101 +/- ##
============================================
- Coverage 63.22% 56.20% -7.03%
+ Complexity 1429 701 -728
============================================
Files 3104 2420 -684
Lines 183356 135357 -47999
Branches 28097 21625 -6472
============================================
- Hits 115930 76072 -39858
+ Misses 58465 52902 -5563
+ Partials 8961 6383 -2578
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/17101/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/17101/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/17101/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/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.20% <84.31%> (-7.01%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.20% <84.31%> (-7.03%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.20% <84.31%> (-7.03%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.20% <84.31%> (-0.06%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17101/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/17101?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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