[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
codecov-io edited a comment on issue #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment URL: https://github.com/apache/incubator-pinot/pull/4533#issuecomment-521871617 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=h1) Report > Merging [#4533](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/4e41bc36b156b8f1d54619eb79fe161eed2c42c9?src=pr=desc) will **decrease** coverage by `13.14%`. > The diff coverage is `0%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/4533/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4533 +/- ## = - Coverage 56.5% 43.35% -13.15% = Files 1075 1073-2 Lines 5594755917 -30 Branches 8168 8182 +14 = - Hits 3161324245 -7368 - Misses2183429519 +7685 + Partials 2500 2153 -347 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ore/assignment/segment/SegmentAssignmentUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9TZWdtZW50QXNzaWdubWVudFV0aWxzLmphdmE=) | `0% <ø> (-96.64%)` | `0 <0> (ø)` | | | [...e/assignment/segment/OfflineSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9PZmZsaW5lU2VnbWVudEFzc2lnbm1lbnQuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | | | [...e/assignment/segment/SegmentAssignmentFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9TZWdtZW50QXNzaWdubWVudEZhY3RvcnkuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | | | [.../assignment/segment/RealtimeSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9SZWFsdGltZVNlZ21lbnRBc3NpZ25tZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | | | [.../org/apache/pinot/common/http/MultiGetRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaHR0cC9NdWx0aUdldFJlcXVlc3QuamF2YQ==) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...che/pinot/common/restlet/resources/TablesList.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVGFibGVzTGlzdC5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...che/pinot/pql/parsers/pql2/ast/OptionsAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PcHRpb25zQXN0Tm9kZS5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...ot/core/query/scheduler/TableBasedGroupMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvVGFibGVCYXNlZEdyb3VwTWFwcGVyLmphdmE=) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...t/core/segment/index/readers/OnHeapDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvT25IZWFwRGljdGlvbmFyeS5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | [...mmon/config/instance/InstanceAssignmentConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL2luc3RhbmNlL0luc3RhbmNlQXNzaWdubWVudENvbmZpZy5qYXZh) | `0% <0%> (-100%)` | `0% <0%> (ø)` | | | ... and [717 more](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=continue). > **Legend** - [Click here to learn
[incubator-pinot] branch segment_assignment updated (387bd4e -> 82372a5)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch segment_assignment in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 387bd4e [Instance Assignment] De-couple assignment strategy from SegmentAssignment add 82372a5 [Instance Assignment] De-couple assignment strategy from SegmentAssignment This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (387bd4e) \ N -- N -- N refs/heads/segment_assignment (82372a5) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../helix/core/assignment/segment/OfflineSegmentAssignment.java | 2 +- .../helix/core/assignment/segment/RealtimeSegmentAssignment.java| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch segment_assignment updated (a20fa2a -> 387bd4e)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch segment_assignment in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard a20fa2a [Instance Assignment] De-couple assignment strategy from SegmentAssignment add 387bd4e [Instance Assignment] De-couple assignment strategy from SegmentAssignment This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (a20fa2a) \ N -- N -- N refs/heads/segment_assignment (387bd4e) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../controller/helix/core/assignment/segment/SegmentAssignment.java | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
Jackie-Jiang commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314918982 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java ## @@ -23,37 +23,42 @@ import org.apache.commons.configuration.Configuration; import org.apache.helix.HelixManager; import org.apache.pinot.common.config.TableConfig; +import org.apache.pinot.common.utils.InstancePartitionsType; +import org.apache.pinot.controller.helix.core.assignment.InstancePartitions; /** - * Strategy to assign segment to instances or rebalance all segments in a table. + * Interface for segment assignment and table rebalancing. */ -public interface SegmentAssignmentStrategy { +public interface SegmentAssignment { Review comment: Good point, added. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
snleee commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314898313 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java ## @@ -23,37 +23,42 @@ import org.apache.commons.configuration.Configuration; import org.apache.helix.HelixManager; import org.apache.pinot.common.config.TableConfig; +import org.apache.pinot.common.utils.InstancePartitionsType; +import org.apache.pinot.controller.helix.core.assignment.InstancePartitions; /** - * Strategy to assign segment to instances or rebalance all segments in a table. + * Interface for segment assignment and table rebalancing. */ -public interface SegmentAssignmentStrategy { +public interface SegmentAssignment { Review comment: Can you add the `TODO` document for adding support for custom segment assignment strategy? I agree that it's not the right time to add the interface since we need the proper design for it. I can see multiple ways to achieve custom segment assignment (e.g. cost model - currently the cost is based on # of segments, we can extend this to add resources like storage factor into the cost). 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
Jackie-Jiang commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314889280 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java ## @@ -23,37 +23,42 @@ import org.apache.commons.configuration.Configuration; import org.apache.helix.HelixManager; import org.apache.pinot.common.config.TableConfig; +import org.apache.pinot.common.utils.InstancePartitionsType; +import org.apache.pinot.controller.helix.core.assignment.InstancePartitions; /** - * Strategy to assign segment to instances or rebalance all segments in a table. + * Interface for segment assignment and table rebalancing. Review comment: This is not the same interface as the one we mentioned in the documentation, but the new added one for the resource assignment feature. We will deprecate the old interface once the new feature is done, and I will add documentation for the new assignment. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4534: Fixing bytes data type merge logic
codecov-io edited a comment on issue #4534: Fixing bytes data type merge logic URL: https://github.com/apache/incubator-pinot/pull/4534#issuecomment-521919418 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=h1) Report > Merging [#4534](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/4e41bc36b156b8f1d54619eb79fe161eed2c42c9?src=pr=desc) will **increase** coverage by `8.3%`. > The diff coverage is `75%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/4534/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree) ```diff @@Coverage Diff @@ ## master #4534 +/- ## == + Coverage 56.5% 64.8% +8.3% Complexity 20 20 == Files 10751075 Lines 55947 55949 +2 Branches 81688168 == + Hits 31613 36258 +4645 + Misses21834 17035 -4799 - Partials 25002656+156 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../operator/transform/TransformBlockDataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vVHJhbnNmb3JtQmxvY2tEYXRhRmV0Y2hlci5qYXZh) | `70.16% <ø> (+46.35%)` | `0 <0> (ø)` | :arrow_down: | | [...t/core/query/selection/SelectionOperatorUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zZWxlY3Rpb24vU2VsZWN0aW9uT3BlcmF0b3JVdGlscy5qYXZh) | `88.05% <75%> (+18.36%)` | `0 <0> (ø)` | :arrow_down: | | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | | | [...nsport/netty/PooledNettyClientResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvUG9vbGVkTmV0dHlDbGllbnRSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `85.41% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [.../org/apache/pinot/transport/netty/NettyServer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvTmV0dHlTZXJ2ZXIuamF2YQ==) | `80.8% <0%> (-3.04%)` | `0% <0%> (ø)` | | | [...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==) | `59.57% <0%> (-2.13%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/BaseOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvQmFzZU9mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `83.92% <0%> (-0.6%)` | `0% <0%> (ø)` | | | [...he/pinot/controller/util/SegmentIntervalUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlZ21lbnRJbnRlcnZhbFV0aWxzLmphdmE=) | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: | | [...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `29.95% <0%> (+0.42%)` | `0% <0%> (ø)` | :arrow_down: | | [...ment/creator/impl/SegmentColumnarIndexCreator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2NyZWF0b3IvaW1wbC9TZWdtZW50Q29sdW1uYXJJbmRleENyZWF0b3IuamF2YQ==) | `77.24% <0%> (+0.59%)` | `0% <0%> (ø)` | :arrow_down: | | ... and [289 more](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ =
[incubator-pinot] branch update_selection_query_1 updated (ecac2e0 -> 905ada5)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch update_selection_query_1 in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard ecac2e0 update column data type cast during inter merge add 905ada5 update column data type cast during inter merge This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (ecac2e0) \ N -- N -- N refs/heads/update_selection_query_1 (905ada5) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../query/selection/SelectionOperatorUtils.java| 4 ++ .../selection/SelectionOperatorServiceTest.java| 46 +++--- 2 files changed, 26 insertions(+), 24 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on issue #4534: Fixing bytes data type merge logic
fx19880617 commented on issue #4534: Fixing bytes data type merge logic URL: https://github.com/apache/incubator-pinot/pull/4534#issuecomment-522063623 Currently we only convert bytes column with dictionary to hex string. However for bytes column without dictionary, the value returned is still byte[]. this will cause cast exception. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
mcvsubbu commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314769606 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java ## @@ -23,37 +23,42 @@ import org.apache.commons.configuration.Configuration; import org.apache.helix.HelixManager; import org.apache.pinot.common.config.TableConfig; +import org.apache.pinot.common.utils.InstancePartitionsType; +import org.apache.pinot.controller.helix.core.assignment.InstancePartitions; /** - * Strategy to assign segment to instances or rebalance all segments in a table. + * Interface for segment assignment and table rebalancing. Review comment: Our documentation states that SegmentAssignmentStrategy is public. You should keep the existing interface and introduce a new one to suit the needs. https://pinot.readthedocs.io/en/latest/customizations.html#segment-assignment-strategies Otherwise it is an incompatible change 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #4534: Fixing bytes data type merge logic
codecov-io commented on issue #4534: Fixing bytes data type merge logic URL: https://github.com/apache/incubator-pinot/pull/4534#issuecomment-521919418 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=h1) Report > Merging [#4534](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/4e41bc36b156b8f1d54619eb79fe161eed2c42c9?src=pr=desc) will **increase** coverage by `8.31%`. > The diff coverage is `0%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/4534/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4534 +/- ## + Coverage 56.5% 64.82% +8.31% Complexity 20 20 Files 1075 1075 Lines 5594755947 Branches 8168 8167 -1 + Hits 3161336265+4652 + Misses2183417021-4813 - Partials 2500 2661 +161 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../operator/transform/TransformBlockDataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vVHJhbnNmb3JtQmxvY2tEYXRhRmV0Y2hlci5qYXZh) | `70.16% <ø> (+46.35%)` | `0 <0> (ø)` | :arrow_down: | | [...t/core/query/selection/SelectionOperatorUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zZWxlY3Rpb24vU2VsZWN0aW9uT3BlcmF0b3JVdGlscy5qYXZh) | `86.84% <0%> (+17.14%)` | `0 <0> (ø)` | :arrow_down: | | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | | | [.../org/apache/pinot/transport/netty/NettyServer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvTmV0dHlTZXJ2ZXIuamF2YQ==) | `80.8% <0%> (-3.04%)` | `0% <0%> (ø)` | | | [...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==) | `59.57% <0%> (-2.13%)` | `0% <0%> (ø)` | | | [...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `29.95% <0%> (+0.42%)` | `0% <0%> (ø)` | :arrow_down: | | [...ment/creator/impl/SegmentColumnarIndexCreator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2NyZWF0b3IvaW1wbC9TZWdtZW50Q29sdW1uYXJJbmRleENyZWF0b3IuamF2YQ==) | `77.24% <0%> (+0.59%)` | `0% <0%> (ø)` | :arrow_down: | | [...r/transform/function/ValueInTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVmFsdWVJblRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | `39.2% <0%> (+0.8%)` | `0% <0%> (ø)` | :arrow_down: | | [.../helix/core/realtime/SegmentCompletionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1NlZ21lbnRDb21wbGV0aW9uTWFuYWdlci5qYXZh) | `70.77% <0%> (+0.86%)` | `0% <0%> (ø)` | :arrow_down: | | [...e/io/writer/impl/MutableOffHeapByteArrayStore.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pby93cml0ZXIvaW1wbC9NdXRhYmxlT2ZmSGVhcEJ5dGVBcnJheVN0b3JlLmphdmE=) | `86.45% <0%> (+1.04%)` | `0% <0%> (ø)` | :arrow_down: | | ... and [288 more](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=continue). > **Legend** - [Click here to
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #4534: Fixing bytes data type merge logic
fx19880617 opened a new pull request #4534: Fixing bytes data type merge logic URL: https://github.com/apache/incubator-pinot/pull/4534 The old implementation convert bytes to string, for dictionary based column, but not for non-dictionary based. Move this logic to merging part. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch update_selection_query_1 created (now ecac2e0)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch update_selection_query_1 in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at ecac2e0 update column data type cast during inter merge This branch includes the following new commits: new ecac2e0 update column data type cast during inter merge The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] 01/01: update column data type cast during inter merge
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a commit to branch update_selection_query_1 in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git commit ecac2e0b57e3b583ee7827adc5a432562217a2c6 Author: Xiang Fu AuthorDate: Thu Aug 15 22:19:08 2019 -0700 update column data type cast during inter merge --- .../pinot/core/operator/transform/TransformBlockDataFetcher.java | 5 + .../apache/pinot/core/query/selection/SelectionOperatorUtils.java| 5 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java index 54e83be..b5c8d99 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java @@ -199,9 +199,6 @@ class DictionaryBasedSVValueFetcher implements Fetcher { } public Serializable getValue(int docId) { -if (_dataType.equals(FieldSpec.DataType.BYTES)) { - return BytesUtils.toHexString(_dictionary.getBytesValue(_dictionaryIds[docId])); -} return (Serializable) _dictionary.get(_dictionaryIds[docId]); } } @@ -337,4 +334,4 @@ class DictionaryBasedMVBytesValueFetcher implements Fetcher { } return values; } -} \ No newline at end of file +} diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java b/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java index bbfa42f..0fe09a5 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java @@ -39,6 +39,7 @@ import org.apache.pinot.common.request.Selection; import org.apache.pinot.common.request.SelectionSort; import org.apache.pinot.common.response.ServerInstance; import org.apache.pinot.common.response.broker.SelectionResults; +import org.apache.pinot.common.utils.BytesUtils; import org.apache.pinot.common.utils.DataSchema; import org.apache.pinot.common.utils.DataTable; import org.apache.pinot.core.common.DataSourceMetadata; @@ -248,9 +249,11 @@ public class SelectionOperatorUtils { dataTableBuilder.setColumn(i, ((Number) columnValue).doubleValue()); break; case STRING: - case BYTES: // BYTES are already converted to String for Selection, before reaching this layer. dataTableBuilder.setColumn(i, ((String) columnValue)); break; + case BYTES: +dataTableBuilder.setColumn(i, BytesUtils.toHexString((byte[]) columnValue)); +break; // Multi-value column. case INT_ARRAY: - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org