phoenix-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] karanmehta93 commented on a change in pull request #419: PHOENIX-4009 Run UPDATE STATISTICS command by using MR integration on…
Date Wed, 09 Jan 2019 18:36:03 GMT
karanmehta93 commented on a change in pull request #419: PHOENIX-4009 Run UPDATE STATISTICS
command by using MR integration on…
URL: https://github.com/apache/phoenix/pull/419#discussion_r246493337
 
 

 ##########
 File path: phoenix-core/src/main/java/org/apache/phoenix/mapreduce/util/PhoenixConfigurationUtil.java
 ##########
 @@ -154,7 +154,8 @@
 
     public enum SchemaType {
         TABLE,
-        QUERY;
+        QUERY,
+        UPDATE_STATS
 
 Review comment:
   I feel that adding a new `SchemaType` is cleaner way to handle this. I agree that everywhere
we check for the `SchemaType`, we need to do for this one as well, but we don't have it in
many places and it provides an insight to the user about the type of MR job it is. If we add
aggregation support in future, this can also be extended similarly. The name `SchemaType`
is not really appropriate name here, however I didn't change it since it is public enum. It
should be something like `PhoenixMRJobType`.
   
   Adding a config property would not eliminate the checks that we have through the code and
I feel that it would make it harder to manipulate in future if new stuff is added. @BinShi-SecularBird
Thoughts?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message