[GitHub] [incubator-pinot] siddharthteotia edited a comment on issue #4527: Group by with order by
siddharthteotia edited a comment on issue #4527: Group by with order by URL: https://github.com/apache/incubator-pinot/pull/4527#issuecomment-521686005 What do people think about implementing ORDER BY as a first class operator in Pinot -- that way different operators and query types will not have to write their own order by functionality. In the current code changes, CombineGroupByOrderByOperator will only combine the results of a query that has group by and order by right? But for the other parallel feature (DISTINCT), we need order by as well and we should probably have a common order by executor and combiner/reducer. With the current code changes, it looks like DISTINCT will have to implement its own ORDER BY? Implementing order by as a first class operator in the operator tree will also work well with plan of getting close to SQL and potentially supporting complex SQL (where each query may have multiple order by's in the physical plan) and an optimizer. 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] npawar commented on a change in pull request #4527: Group by with order by
npawar commented on a change in pull request #4527: Group by with order by URL: https://github.com/apache/incubator-pinot/pull/4527#discussion_r314383656 ## File path: pinot-common/src/main/java/org/apache/pinot/common/response/broker/OrderedGroupByResults.java ## @@ -28,39 +28,26 @@ /** * This class holds the results of group by order by query which is set into the {@link BrokerResponseNative} */ -@JsonPropertyOrder({"orderBy", "groupByKeys", "aggregationResults"}) -public class GroupByOrderByResults { - private List _orderBy; - private List _groupByKeys; +@JsonPropertyOrder({"groupBy", "aggregationResults"}) +public class OrderedGroupByResults { Review comment: Does not.. but we will have continue to keep the current group by behavior 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. 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] kishoreg commented on a change in pull request #4527: Group by with order by
kishoreg commented on a change in pull request #4527: Group by with order by URL: https://github.com/apache/incubator-pinot/pull/4527#discussion_r314147558 ## File path: pinot-common/src/main/java/org/apache/pinot/common/response/broker/OrderedGroupByResults.java ## @@ -28,39 +28,26 @@ /** * This class holds the results of group by order by query which is set into the {@link BrokerResponseNative} */ -@JsonPropertyOrder({"orderBy", "groupByKeys", "aggregationResults"}) -public class GroupByOrderByResults { - private List _orderBy; - private List _groupByKeys; +@JsonPropertyOrder({"groupBy", "aggregationResults"}) +public class OrderedGroupByResults { Review comment: Why does it matter if it's ordered or not? 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] kishoreg commented on a change in pull request #4527: Group by with order by
kishoreg commented on a change in pull request #4527: Group by with order by URL: https://github.com/apache/incubator-pinot/pull/4527#discussion_r314147427 ## File path: pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java ## @@ -64,7 +64,7 @@ private SelectionResults _selectionResults; private List _aggregationResults; - private GroupByOrderByResults _groupByOrderByResults; + private OrderedGroupByResults _orderedGroupByResults; Review comment: why do we need another element in the response? can we not use the existing structure 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 #4323: [Controller Separation] Add logic for lead controller resource
codecov-io edited a comment on issue #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323#issuecomment-502327426 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4323?src=pr=h1) Report > Merging [#4323](https://codecov.io/gh/apache/incubator-pinot/pull/4323?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/84464afa71b475e0ce79c10359526c5bf39f0501?src=pr=desc) will **decrease** coverage by `0.04%`. > The diff coverage is `84.42%`. [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/4323/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4323?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4323 +/- ## - Coverage 64.75% 64.71% -0.05% Complexity 20 20 Files 1073 1074 +1 Lines 5570455793 +89 Branches 8115 8127 +12 + Hits 3607136104 +33 - Misses1697917052 +73 + Partials 2654 2637 -17 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4323?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.4% <ø> (-0.04%)` | `0 <0> (ø)` | | | [...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `42.3% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: | | [.../pinot/controller/api/upload/SegmentValidator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvdXBsb2FkL1NlZ21lbnRWYWxpZGF0b3IuamF2YQ==) | `64.7% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...t/core/data/partition/MurmurPartitionFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3BhcnRpdGlvbi9NdXJtdXJQYXJ0aXRpb25GdW5jdGlvbi5qYXZh) | `77.77% <100%> (-15.78%)` | `0 <0> (ø)` | | | [...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=) | `79.16% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...anager/realtime/SegmentBuildTimeLeaseExtender.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudEJ1aWxkVGltZUxlYXNlRXh0ZW5kZXIuamF2YQ==) | `57.4% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `81.96% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `70.87% <100%> (-0.32%)` | `0 <0> (ø)` | | | [...n/java/org/apache/pinot/common/utils/HashUtil.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvSGFzaFV0aWwuamF2YQ==) | `43.1% <100%> (+37.38%)` | `0 <0> (ø)` | :arrow_down: | | [...ontrollerResourceMasterSlaveStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3N0YXRlbW9kZWwvTGVhZENvbnRyb2xsZXJSZXNvdXJjZU1hc3RlclNsYXZlU3RhdGVNb2RlbEZhY3RvcnkuamF2YQ==) | `100% <100%> (ø)` | `0 <0> (?)` | | | ... and [52 more](https://codecov.io/gh/apache/incubator-pinot/pull/4323/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] siddharthteotia commented on issue #4527: Group by with order by
siddharthteotia commented on issue #4527: Group by with order by URL: https://github.com/apache/incubator-pinot/pull/4527#issuecomment-521686005 What do people think about implementing ORDER BY as a first class operator in Pinot -- that way different operators and query types will not have to write their own order by functionality. In the current code changes, CombineGroupByOrderByOperator will only combine the results of a query that has group by and order by right? But for the other parallel feature (DISTINCT), we need order by as well and we should probably have a common order by executor and combiner/reducer. Implementing order by as a first class operator in the operator tree will also work well with plan of getting close to SQL and potentially supporting complex SQL (where each query may have multiple order by's in the physical plan) and an optimizer. 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] sunithabeeram opened a new pull request #4532: Expose a method to determine if a QueryExceptionErrorCode represents a client-side error
sunithabeeram opened a new pull request #4532: Expose a method to determine if a QueryExceptionErrorCode represents a client-side error URL: https://github.com/apache/incubator-pinot/pull/4532 Add a method in the QueryException class to determine if an error code represents a client-side error. Adding it in the class directly helps keep the method up-to-date when new codes are added. Also, expose the value used by RequestStatistics object to indicate the default value set for tables so higher layers can use it appropriately to determine whether the table name was parsed correctly or not. 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] npawar commented on a change in pull request #4527: Group by with order by
npawar commented on a change in pull request #4527: Group by with order by URL: https://github.com/apache/incubator-pinot/pull/4527#discussion_r314382962 ## File path: pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java ## @@ -64,7 +64,7 @@ private SelectionResults _selectionResults; private List _aggregationResults; - private GroupByOrderByResults _groupByOrderByResults; + private OrderedGroupByResults _orderedGroupByResults; Review comment: There's 2 options in the existing structure: 1. Selection results - this has only selection columns 2. List - these are split up as group by columns + 1 aggregation, per aggregation Potentially we can get rid of List when we completely remove the existing group by behavior 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 #4516: [Instance Assignment] Plug in instance assignment
snleee commented on a change in pull request #4516: [Instance Assignment] Plug in instance assignment URL: https://github.com/apache/incubator-pinot/pull/4516#discussion_r314441546 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java ## @@ -1130,43 +1134,6 @@ void validateTableTenantConfig(TableConfig tableConfig) { } } - /** - * Update replica group partition assignment in the property store - * - * @param tableConfig a table config - */ - private void updateReplicaGroupPartitionAssignment(TableConfig tableConfig) { -String tableNameWithType = tableConfig.getTableName(); -String assignmentStrategy = tableConfig.getValidationConfig().getSegmentAssignmentStrategy(); -// We create replica group partition assignment and write to property store if new table config -// has the replica group config. -if (assignmentStrategy != null && SegmentAssignmentStrategyEnum.valueOf(assignmentStrategy) -== SegmentAssignmentStrategyEnum.ReplicaGroupSegmentAssignmentStrategy) { - ReplicaGroupPartitionAssignmentGenerator partitionAssignmentGenerator = Review comment: We can remove this 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. 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 add-logic-for-lead-controller-resource deleted (was 92ae1ab)
This is an automated email from the ASF dual-hosted git repository. jlli pushed a change to branch add-logic-for-lead-controller-resource in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. was 92ae1ab Address PR comments The revisions that were on this branch are still contained in other references; therefore, this change does not discard any commits from the repository. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli merged pull request #4323: [Controller Separation] Add logic for lead controller resource
jackjlli merged pull request #4323: [Controller Separation] Add logic for lead controller resource URL: https://github.com/apache/incubator-pinot/pull/4323 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] sunithabeeram closed pull request #4356: Emit metrics to track performance against specific Service-Level-Objectives (SLO)
sunithabeeram closed pull request #4356: Emit metrics to track performance against specific Service-Level-Objectives (SLO) URL: https://github.com/apache/incubator-pinot/pull/4356 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] sunithabeeram closed pull request #4380: Define SLO config for tables
sunithabeeram closed pull request #4380: Define SLO config for tables URL: https://github.com/apache/incubator-pinot/pull/4380 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] sunithabeeram commented on issue #4356: Emit metrics to track performance against specific Service-Level-Objectives (SLO)
sunithabeeram commented on issue #4356: Emit metrics to track performance against specific Service-Level-Objectives (SLO) URL: https://github.com/apache/incubator-pinot/pull/4356#issuecomment-521829273 Closing this as the logic has been moved to higher layers. 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] sunithabeeram commented on issue #4380: Define SLO config for tables
sunithabeeram commented on issue #4380: Define SLO config for tables URL: https://github.com/apache/incubator-pinot/pull/4380#issuecomment-521829184 Closing this as this has been moved to higher layers. 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 plug_in_instance_assignment deleted (was 18cba13)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch plug_in_instance_assignment in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. was 18cba13 [Instance Assignment] Plug in instance assignment The revisions that were on this branch are still contained in other references; therefore, this change does not discard any commits from the repository. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: [Instance Assignment] Plug in instance assignment (#4516)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git The following commit(s) were added to refs/heads/master by this push: new 4e41bc3 [Instance Assignment] Plug in instance assignment (#4516) 4e41bc3 is described below commit 4e41bc36b156b8f1d54619eb79fe161eed2c42c9 Author: Xiaotian (Jackie) Jiang <1751+jackie-ji...@users.noreply.github.com> AuthorDate: Thu Aug 15 17:29:41 2019 -0700 [Instance Assignment] Plug in instance assignment (#4516) - Add instance assignment APIs including: - GET/PUT/DELETE instance partitions - Assign instances for a table - Replace instance in instance partitions - Assign instances if necessary while creating/updating table config - Backward-compatible, no change to the cluster if instance assignment is not configured --- .../instance/InstanceAssignmentConfigUtils.java| 84 ++--- .../pinot/common/metadata/ZKMetadataProvider.java | 8 - .../PinotInstanceAssignmentRestletResource.java| 301 ++ .../helix/ControllerRequestURLBuilder.java | 38 ++- .../helix/core/PinotHelixResourceManager.java | 183 +-- .../helix/core/assignment/InstancePartitions.java | 8 +- .../core/assignment/InstancePartitionsUtils.java | 56 +++- .../instance/InstanceAssignmentDriver.java | 6 +- .../instance/InstanceReplicaPartitionSelector.java | 7 +- ...PinotInstanceAssignmentRestletResourceTest.java | 339 + .../instance/InstanceAssignmentTest.java | 93 +- .../ReplicaGroupRebalanceStrategyTest.java | 4 +- .../sharding/SegmentAssignmentStrategyTest.java| 72 - .../apache/pinot/core/util/ReplicationUtils.java | 35 --- 14 files changed, 902 insertions(+), 332 deletions(-) diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/instance/InstanceAssignmentConfigUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/config/instance/InstanceAssignmentConfigUtils.java index f094527..446f107 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/config/instance/InstanceAssignmentConfigUtils.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/config/instance/InstanceAssignmentConfigUtils.java @@ -33,8 +33,40 @@ public class InstanceAssignmentConfigUtils { private InstanceAssignmentConfigUtils() { } + /** + * Returns whether the instance assignment is allowed for the given table config. + */ + public static boolean allowInstanceAssignment(TableConfig tableConfig, + InstancePartitionsType instancePartitionsType) { +TableType tableType = tableConfig.getTableType(); +Map instanceAssignmentConfigMap = +tableConfig.getInstanceAssignmentConfigMap(); +switch (instancePartitionsType) { + // Allow OFFLINE instance assignment if the OFFLINE table has it configured or (for backward-compatibility) is + // using replica-group segment assignment + case OFFLINE: +return tableType == TableType.OFFLINE && ((instanceAssignmentConfigMap != null && instanceAssignmentConfigMap +.containsKey(InstancePartitionsType.OFFLINE)) +|| AssignmentStrategy.REPLICA_GROUP_SEGMENT_ASSIGNMENT_STRATEGY + .equalsIgnoreCase(tableConfig.getValidationConfig().getSegmentAssignmentStrategy())); + // Allow CONSUMING/COMPLETED instance assignment if the REALTIME table has it configured + case CONSUMING: + case COMPLETED: +return tableType == TableType.REALTIME && (instanceAssignmentConfigMap != null && instanceAssignmentConfigMap +.containsKey(instancePartitionsType)); + default: +throw new IllegalStateException(); +} + } + + /** + * Extracts or generates default instance assignment config from the given table config. + */ public static InstanceAssignmentConfig getInstanceAssignmentConfig(TableConfig tableConfig, InstancePartitionsType instancePartitionsType) { +Preconditions.checkState(allowInstanceAssignment(tableConfig, instancePartitionsType), +"Instance assignment is not allowed for the given table config"); + // Use the instance assignment config from the table config if it exists Map instanceAssignmentConfigMap = tableConfig.getInstanceAssignmentConfigMap(); @@ -42,49 +74,33 @@ public class InstanceAssignmentConfigUtils { if (instanceAssignmentConfigMap.containsKey(instancePartitionsType)) { return instanceAssignmentConfigMap.get(instancePartitionsType); } - // Use the CONSUMING config for COMPLETED segments if COMPLETED config does not exist - if (instancePartitionsType == InstancePartitionsType.COMPLETED && instanceAssignmentConfigMap - .containsKey(InstancePartitionsType.CONSUMING)) { -return
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #4516: [Instance Assignment] Plug in instance assignment
Jackie-Jiang merged pull request #4516: [Instance Assignment] Plug in instance assignment URL: https://github.com/apache/incubator-pinot/pull/4516 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] haibow commented on issue #4225: Make Pinot schema evolution easier
haibow commented on issue #4225: Make Pinot schema evolution easier URL: https://github.com/apache/incubator-pinot/issues/4225#issuecomment-521824275 Seems reload doesn't work well with REALTIME tables, and the table will end up having segments with inconsistent schemas. For realtime tables, after updating the schema and calling `reload` endpoint, all ONLINE segments would be reloaded with the new schema, but the CONSUMING segment would be skipped. As a result, the consuming segment would both keep consuming and finally seal with the old schema. Tested on a realtime table with LLC (code last checked in from master on [04/11/19](https://github.com/apache/incubator-pinot/commit/26330f3a2c3309e7cf574e1fff86a1de9fb934ff)). The consuming segment at the time of the reloading would be the only segment with the old schema, when either in CONSUMING state or later in ONLINE state. Impact: - when querying data within a small time range after the time of reloading, the new field added in the new schema is not returned in the query result. - when querying data with a bit bigger time range, we would see messages below: `MergeResponseError: responses for table: $table from servers: [$server1, server2] got dropped due to data schema inconsistency.` Reloading the table/segment again after the consuming segment seals would reload it with the new schema thus bringing the whole table back in healthy state, but it's operationally inefficient. So it seems more like a bug now. We might need to revisit approaches like - flushing the consuming segment (and reload) - adding new columns in memory, and refresh the schema, before consuming more rows (without a forced flush) @mayankshriv @Jackie-Jiang thoughts? 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 #4516: [Instance Assignment] Plug in instance assignment
Jackie-Jiang commented on a change in pull request #4516: [Instance Assignment] Plug in instance assignment URL: https://github.com/apache/incubator-pinot/pull/4516#discussion_r314475279 ## File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java ## @@ -1130,43 +1134,6 @@ void validateTableTenantConfig(TableConfig tableConfig) { } } - /** - * Update replica group partition assignment in the property store - * - * @param tableConfig a table config - */ - private void updateReplicaGroupPartitionAssignment(TableConfig tableConfig) { -String tableNameWithType = tableConfig.getTableName(); -String assignmentStrategy = tableConfig.getValidationConfig().getSegmentAssignmentStrategy(); -// We create replica group partition assignment and write to property store if new table config -// has the replica group config. -if (assignmentStrategy != null && SegmentAssignmentStrategyEnum.valueOf(assignmentStrategy) -== SegmentAssignmentStrategyEnum.ReplicaGroupSegmentAssignmentStrategy) { - ReplicaGroupPartitionAssignmentGenerator partitionAssignmentGenerator = Review comment: Will do after plugging in the segment assignment 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 segment_assignment created (now 571e14a)
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. at 571e14a De-couple assignment strategy from SegmentAssignment This branch includes the following new commits: new 571e14a De-couple assignment strategy from SegmentAssignment 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: De-couple assignment strategy from SegmentAssignment
This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch segment_assignment in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git commit 571e14a992947419c73b6088cfbc5a4095fa6c22 Author: Jackie (Xiaotian) Jiang AuthorDate: Thu Aug 15 19:20:59 2019 -0700 De-couple assignment strategy from SegmentAssignment For REALTIME segments, we should be able to assign CONSUMING and COMPLETED segments with different strategies, i.e. one could follows replica-group while the other follows balance-number. In order to do so, we need to de-couple the assignment strategy from the segment assignment, so each assignment can use different strategies if necessary. - Change SegmentAssignmentStrategy interface to SegmentAssignment - Add OfflineSegmentAssignment and RealtimeSegmentAssignment - Use InstancePartitions to determine the strategy to use - For REALTIME table, support different strategies for CONSUMING and COMPLETED segments --- ...OfflineBalanceNumSegmentAssignmentStrategy.java | 96 ...flineReplicaGroupSegmentAssignmentStrategy.java | 200 --- .../segment/OfflineSegmentAssignment.java | 268 + ...ealtimeBalanceNumSegmentAssignmentStrategy.java | 161 - ...ltimeReplicaGroupSegmentAssignmentStrategy.java | 165 - .../segment/RealtimeSegmentAssignment.java | 216 + ...ignmentStrategy.java => SegmentAssignment.java} | 23 +- .../segment/SegmentAssignmentFactory.java | 43 .../segment/SegmentAssignmentStrategyFactory.java | 54 - .../assignment/segment/SegmentAssignmentUtils.java | 21 -- ...flineNonReplicaGroupSegmentAssignmentTest.java} | 43 ++-- ... OfflineReplicaGroupSegmentAssignmentTest.java} | 115 - ...ltimeNonReplicaGroupSegmentAssignmentTest.java} | 65 ++--- ...RealtimeReplicaGroupSegmentAssignmentTest.java} | 67 ++ ...trategyTest.java => SegmentAssignmentTest.java} | 2 +- 15 files changed, 666 insertions(+), 873 deletions(-) diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineBalanceNumSegmentAssignmentStrategy.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineBalanceNumSegmentAssignmentStrategy.java deleted file mode 100644 index 4bd34b5..000 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineBalanceNumSegmentAssignmentStrategy.java +++ /dev/null @@ -1,96 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.pinot.controller.helix.core.assignment.segment; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.PriorityQueue; -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.common.utils.Pairs; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - - -/** - * Segment assignment strategy for offline segments that assigns segment to the instance with the least number of - * segments. In case of a tie, assigns to the instance with the smallest index in the list. The strategy ensures that - * replicas of the same segment are not assigned to the same server. - * To rebalance a table, use Helix AutoRebalanceStrategy. - */ -public class OfflineBalanceNumSegmentAssignmentStrategy implements SegmentAssignmentStrategy { - private static final Logger LOGGER = LoggerFactory.getLogger(OfflineBalanceNumSegmentAssignmentStrategy.class); - - private HelixManager _helixManager; - private TableConfig _tableConfig; - private String _tableNameWithType; - private int _replication; - - @Override - public void init(HelixManager helixManager, TableConfig tableConfig) { -_helixManager = helixManager; -_tableConfig = tableConfig; -_tableNameWithType = tableConfig.getTableName(); -_replication = tableConfig.getValidationConfig().getReplicationNumber(); - -
[incubator-pinot] 01/01: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch segment_assignment in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git commit a9e32982ce460625e95100d241e5ddcad5cc2edf Author: Jackie (Xiaotian) Jiang AuthorDate: Thu Aug 15 19:20:59 2019 -0700 [Instance Assignment] De-couple assignment strategy from SegmentAssignment For REALTIME segments, we should be able to assign CONSUMING and COMPLETED segments with different strategies, i.e. one could follows replica-group while the other follows balance-number. In order to do so, we need to de-couple the assignment strategy from the segment assignment, so each assignment can use different strategies if necessary. - Change SegmentAssignmentStrategy interface to SegmentAssignment - Add OfflineSegmentAssignment and RealtimeSegmentAssignment - Use InstancePartitions to determine the strategy to use - For REALTIME table, support different strategies for CONSUMING and COMPLETED segments --- ...OfflineBalanceNumSegmentAssignmentStrategy.java | 96 ...flineReplicaGroupSegmentAssignmentStrategy.java | 200 --- .../segment/OfflineSegmentAssignment.java | 268 + ...ealtimeBalanceNumSegmentAssignmentStrategy.java | 161 - ...ltimeReplicaGroupSegmentAssignmentStrategy.java | 165 - .../segment/RealtimeSegmentAssignment.java | 216 + ...ignmentStrategy.java => SegmentAssignment.java} | 23 +- .../segment/SegmentAssignmentFactory.java | 43 .../segment/SegmentAssignmentStrategyFactory.java | 54 - .../assignment/segment/SegmentAssignmentUtils.java | 21 -- ...flineNonReplicaGroupSegmentAssignmentTest.java} | 43 ++-- ... OfflineReplicaGroupSegmentAssignmentTest.java} | 115 - ...ltimeNonReplicaGroupSegmentAssignmentTest.java} | 65 ++--- ...RealtimeReplicaGroupSegmentAssignmentTest.java} | 67 ++ ...trategyTest.java => SegmentAssignmentTest.java} | 2 +- 15 files changed, 666 insertions(+), 873 deletions(-) diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineBalanceNumSegmentAssignmentStrategy.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineBalanceNumSegmentAssignmentStrategy.java deleted file mode 100644 index 4bd34b5..000 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineBalanceNumSegmentAssignmentStrategy.java +++ /dev/null @@ -1,96 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.pinot.controller.helix.core.assignment.segment; - -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.PriorityQueue; -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.common.utils.Pairs; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - - -/** - * Segment assignment strategy for offline segments that assigns segment to the instance with the least number of - * segments. In case of a tie, assigns to the instance with the smallest index in the list. The strategy ensures that - * replicas of the same segment are not assigned to the same server. - * To rebalance a table, use Helix AutoRebalanceStrategy. - */ -public class OfflineBalanceNumSegmentAssignmentStrategy implements SegmentAssignmentStrategy { - private static final Logger LOGGER = LoggerFactory.getLogger(OfflineBalanceNumSegmentAssignmentStrategy.class); - - private HelixManager _helixManager; - private TableConfig _tableConfig; - private String _tableNameWithType; - private int _replication; - - @Override - public void init(HelixManager helixManager, TableConfig tableConfig) { -_helixManager = helixManager; -_tableConfig = tableConfig; -_tableNameWithType = tableConfig.getTableName(); -_replication = tableConfig.getValidationConfig().getReplicationNumber();
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
Jackie-Jiang opened a new pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment URL: https://github.com/apache/incubator-pinot/pull/4533 For REALTIME segments, we should be able to assign CONSUMING and COMPLETED segments with different strategies, i.e. one could follows replica-group while the other follows balance-number. In order to do so, we need to de-couple the assignment strategy from the segment assignment, so each assignment can use different strategies if necessary. - Change SegmentAssignmentStrategy interface to SegmentAssignment - Add OfflineSegmentAssignment and RealtimeSegmentAssignment - Use InstancePartitions to determine the strategy to use - For REALTIME table, support different strategies for CONSUMING and COMPLETED segments 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 segment_assignment updated (571e14a -> a9e3298)
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 571e14a De-couple assignment strategy from SegmentAssignment new a9e3298 [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 (571e14a) \ N -- N -- N refs/heads/segment_assignment (a9e3298) 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. 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. Summary of changes: - 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 (a9e3298 -> a20fa2a)
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 a9e3298 [Instance Assignment] De-couple assignment strategy from SegmentAssignment add a20fa2a [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 (a9e3298) \ N -- N -- N refs/heads/segment_assignment (a20fa2a) 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: .../{SegmentAssignmentTest.java => SegmentAssignmentStrategyTest.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/sharding/{SegmentAssignmentTest.java => SegmentAssignmentStrategyTest.java} (99%) - 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 #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment
codecov-io commented 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 **increase** coverage by `8.21%`. > The diff coverage is `87.74%`. [![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% 64.72% +8.21% Complexity 20 20 Files 1075 1073 -2 Lines 5594755917 -30 Branches 8168 8182 +14 + Hits 3161336190+4577 + Misses2183417071-4763 - Partials 2500 2656 +156 ``` | [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=) | `98.18% <ø> (+1.54%)` | `0 <0> (ø)` | :arrow_down: | | [...e/assignment/segment/SegmentAssignmentFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9TZWdtZW50QXNzaWdubWVudEZhY3RvcnkuamF2YQ==) | `71.42% <71.42%> (ø)` | `0 <0> (?)` | | | [.../assignment/segment/RealtimeSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9SZWFsdGltZVNlZ21lbnRBc3NpZ25tZW50LmphdmE=) | `86.58% <86.58%> (ø)` | `0 <0> (?)` | | | [...e/assignment/segment/OfflineSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9PZmZsaW5lU2VnbWVudEFzc2lnbm1lbnQuamF2YQ==) | `89.56% <89.56%> (ø)` | `0 <0> (?)` | | | [...helix/core/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvSW5zdGFuY2VQYXJ0aXRpb25zVXRpbHMuamF2YQ==) | `20.58% <0%> (-17.65%)` | `0% <0%> (ø)` | | | [...nsport/netty/PooledNettyClientResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvUG9vbGVkTmV0dHlDbGllbnRSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `85.41% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/BaseOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvQmFzZU9mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `83.92% <0%> (-0.6%)` | `0% <0%> (ø)` | | | ... and [292 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 more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=footer). Last update [4e41bc3...a20fa2a](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). 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
[incubator-pinot] branch update_selection_query created (now 39c91cf)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch update_selection_query in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at 39c91cf update column data type cast during inter merge This branch includes the following new commits: new 39c91cf 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 in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git commit 39c91cf7aa6f7bd3d305576474783779f8fc8363 Author: Xiang Fu AuthorDate: Thu Aug 15 22:19:08 2019 -0700 update column data type cast during inter merge --- .../apache/pinot/core/query/selection/SelectionOperatorUtils.java | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) 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..42d3e64 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; @@ -249,7 +250,11 @@ public class SelectionOperatorUtils { break; case STRING: case BYTES: // BYTES are already converted to String for Selection, before reaching this layer. -dataTableBuilder.setColumn(i, ((String) columnValue)); +if (columnValue instanceof byte[]) { + dataTableBuilder.setColumn(i, BytesUtils.toHexString((byte[]) columnValue)); +} else { + dataTableBuilder.setColumn(i, ((String) columnValue)); +} break; // Multi-value column. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org