Re: Review Request 24137: allow disabling direct sql per query with external metastore

2014-08-11 Thread Navis Ryu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/
---

(Updated Aug. 11, 2014, 7:37 a.m.)


Review request for hive.


Changes
---

added getMetaConf(), which shows current value of the meta variable.


Bugs: HIVE-7532
https://issues.apache.org/jira/browse/HIVE-7532


Repository: hive-git


Description
---

Currently with external metastore, direct sql can only be disabled via 
metastore config globally. Perhaps it makes sense to have the ability to 
propagate the setting per query from client to override the metastore setting, 
e.g. if one particular query causes it to fail.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8490558 
  common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
 9e416b5 
  metastore/if/hive_metastore.thrift 9e93b95 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
6e689d0 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
85a77d9 
  metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 1675751 
  metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
8746c37 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java 
c28c46a 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
86172b9 
  
metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 39b032e 
  ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java 2baa24a 
  ql/src/test/queries/clientpositive/set_metaconf.q PRE-CREATION 
  ql/src/test/results/clientpositive/set_metaconf.q.out PRE-CREATION 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 
4c3164e 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 
b39d64d 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 
c2f0495 

Diff: https://reviews.apache.org/r/24137/diff/


Testing
---


Thanks,

Navis Ryu

Re: Review Request 24137: allow disabling direct sql per query with external metastore

2014-08-03 Thread Navis Ryu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/
---

(Updated Aug. 4, 2014, 2:12 a.m.)


Review request for hive.


Changes
---

Addressed comments


Bugs: HIVE-7532
https://issues.apache.org/jira/browse/HIVE-7532


Repository: hive-git


Description
---

Currently with external metastore, direct sql can only be disabled via 
metastore config globally. Perhaps it makes sense to have the ability to 
propagate the setting per query from client to override the metastore setting, 
e.g. if one particular query causes it to fail.


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 15bc0a3 
  common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
 9e416b5 
  metastore/if/hive_metastore.thrift 55f41db 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
b74868b 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
4c9a597 
  metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 1675751 
  metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
d6e849f 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java 
c28c46a 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
86172b9 
  
metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad 
  ql/src/java/org/apache/hadoop/hive/ql/processors/SetProcessor.java 2baa24a 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 
4c3164e 
  service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 
b39d64d 
  service/src/java/org/apache/hive/service/cli/session/SessionManager.java 
c2f0495 

Diff: https://reviews.apache.org/r/24137/diff/


Testing
---


Thanks,

Navis Ryu

Re: Review Request 24137: allow disabling direct sql per query with external metastore

2014-08-03 Thread Navis Ryu 


> On Aug. 1, 2014, 9:35 a.m., Lars Francke wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 71
> > 
> >
> > Should be named `META_CONFS`

There are fields something like "vars" and "metaVars", which is static. I also 
think uppercase names are better for static fields, but also feel that 
consistency is more important.


> On Aug. 1, 2014, 9:35 a.m., Lars Francke wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 161
> > 
> >
> > Should be named `META_CONF_VARS`
> > 
> > Also all the `HiveConf` qualifiers are not necessary
> > 
> > Indentation is 2 not 4 as per the Wiki

Same as above.


> On Aug. 1, 2014, 9:35 a.m., Lars Francke wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java,
> >  line 63
> > 
> >
> > no need to call getConf() here

To make that same form with other invocations in the class. Just for 
consistency.


- Navis


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/#review49330
---


On July 31, 2014, 1:34 a.m., Navis Ryu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24137/
> ---
> 
> (Updated July 31, 2014, 1:34 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-7532
> https://issues.apache.org/jira/browse/HIVE-7532
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently with external metastore, direct sql can only be disabled via 
> metastore config globally. Perhaps it makes sense to have the ability to 
> propagate the setting per query from client to override the metastore 
> setting, e.g. if one particular query causes it to fail.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3bfc681 
>   common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
>  9e416b5 
>   metastore/if/hive_metastore.thrift 55f41db 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 5cc1cd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> d26183b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 
> 1675751 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> 5add436 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
>  c28c46a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java 
> 1cf09d4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 86172b9 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Re: Review Request 24137: allow disabling direct sql per query with external metastore

2014-08-01 Thread Lars Francke 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/#review49330
---


Code style issues only


common/src/java/org/apache/hadoop/hive/conf/HiveConf.java


Should be named `META_CONFS`



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java


Should be named `META_CONF_VARS`

Also all the `HiveConf` qualifiers are not necessary

Indentation is 2 not 4 as per the Wiki



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


Line too long



metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java


extra space between extends and ThriftHiveMetastore.Iface



metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java


`public` is not needed in interfaces



metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java


Javadoc is wrong



metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java


no need to call getConf() here



metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java


line too long



metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java


line too long


- Lars Francke


On July 31, 2014, 1:34 a.m., Navis Ryu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24137/
> ---
> 
> (Updated July 31, 2014, 1:34 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-7532
> https://issues.apache.org/jira/browse/HIVE-7532
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently with external metastore, direct sql can only be disabled via 
> metastore config globally. Perhaps it makes sense to have the ability to 
> propagate the setting per query from client to override the metastore 
> setting, e.g. if one particular query causes it to fail.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3bfc681 
>   common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
>  9e416b5 
>   metastore/if/hive_metastore.thrift 55f41db 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 5cc1cd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> d26183b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 
> 1675751 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> 5add436 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
>  c28c46a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java 
> 1cf09d4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 86172b9 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Re: Review Request 24137: allow disabling direct sql per query with external metastore

2014-07-31 Thread Navis Ryu 


> On July 31, 2014, 9:01 p.m., Sergey Shelukhin wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, 
> > line 395
> > 
> >
> > would it be enough to make it package-visible?

It's on IHMSHandler, which is a public interface. 


> On July 31, 2014, 9:01 p.m., Sergey Shelukhin wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, 
> > line 270
> > 
> >
> > why are these made static? I see that there can be multiple 
> > HMSHandler-s, incl. thru derived classes. Are they always one per thread? 
> > If multiple handlers (incl. derived ones), semantics can change

I've been misunderstood till now that the HMSHandler is created per client like 
hiveservers. Cannot imagine there would be a case that a client is using 
multiple processors simultaneously but I've understood your point. Will be 
rolled-back.


- Navis


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/#review49290
---


On July 31, 2014, 1:34 a.m., Navis Ryu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24137/
> ---
> 
> (Updated July 31, 2014, 1:34 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-7532
> https://issues.apache.org/jira/browse/HIVE-7532
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently with external metastore, direct sql can only be disabled via 
> metastore config globally. Perhaps it makes sense to have the ability to 
> propagate the setting per query from client to override the metastore 
> setting, e.g. if one particular query causes it to fail.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3bfc681 
>   common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
>  9e416b5 
>   metastore/if/hive_metastore.thrift 55f41db 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 5cc1cd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> d26183b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 
> 1675751 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> 5add436 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
>  c28c46a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java 
> 1cf09d4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 86172b9 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Re: Review Request 24137: allow disabling direct sql per query with external metastore

2014-07-31 Thread Sergey Shelukhin 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/#review49290
---



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


why are these made static? I see that there can be multiple HMSHandler-s, 
incl. thru derived classes. Are they always one per thread? If multiple 
handlers (incl. derived ones), semantics can change



metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java


would it be enough to make it package-visible?


- Sergey Shelukhin


On July 31, 2014, 1:34 a.m., Navis Ryu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24137/
> ---
> 
> (Updated July 31, 2014, 1:34 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-7532
> https://issues.apache.org/jira/browse/HIVE-7532
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Currently with external metastore, direct sql can only be disabled via 
> metastore config globally. Perhaps it makes sense to have the ability to 
> propagate the setting per query from client to override the metastore 
> setting, e.g. if one particular query causes it to fail.
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3bfc681 
>   common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
>  9e416b5 
>   metastore/if/hive_metastore.thrift 55f41db 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 5cc1cd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> d26183b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 
> 1675751 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> 5add436 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java
>  c28c46a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java 
> 1cf09d4 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
> 86172b9 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/24137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>

Review Request 24137: allow disabling direct sql per query with external metastore

2014-07-30 Thread Navis Ryu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24137/
---

Review request for hive.


Bugs: HIVE-7532
https://issues.apache.org/jira/browse/HIVE-7532


Repository: hive-git


Description
---

Currently with external metastore, direct sql can only be disabled via 
metastore config globally. Perhaps it makes sense to have the ability to 
propagate the setting per query from client to override the metastore setting, 
e.g. if one particular query causes it to fail.


Diffs
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 3bfc681 
  common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java ee98d17 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
 9e416b5 
  metastore/if/hive_metastore.thrift 55f41db 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
5cc1cd8 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
d26183b 
  metastore/src/java/org/apache/hadoop/hive/metastore/IHMSHandler.java 1675751 
  metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
5add436 
  
metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreEventListener.java 
c28c46a 
  metastore/src/java/org/apache/hadoop/hive/metastore/RawStoreProxy.java 
1cf09d4 
  metastore/src/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java 
86172b9 
  
metastore/src/java/org/apache/hadoop/hive/metastore/events/ConfigChangeEvent.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/24137/diff/


Testing
---


Thanks,

Navis Ryu