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:
[email protected]
With regards,
Apache Git Services