[GitHub] [incubator-pinot] siddharthteotia edited a comment on issue #4527: Group by with order by

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread jlli
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread jackie
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)

2019-08-15 Thread jackie
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread jackie
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

2019-08-15 Thread jackie
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

2019-08-15 Thread jackie
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread jackie
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)

2019-08-15 Thread jackie
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

2019-08-15 Thread GitBox
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)

2019-08-15 Thread xiangfu
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

2019-08-15 Thread xiangfu
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