----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48233/#review136810 -----------------------------------------------------------
Fix it, then Ship it! Mostly minor cleanup nitpicks. Might make sense in the future to refactor this into a separate class that handles this sort of check, but this is fine for now. Fix then ship. metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java (line 3131) <https://reviews.apache.org/r/48233/#comment201896> Can we create a 'public static final String' for this instead of using a comment? metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 2523) <https://reviews.apache.org/r/48233/#comment201897> Nit: Strange extra space at the end, is that needed? metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 2830) <https://reviews.apache.org/r/48233/#comment201898> Maybe StringUtils.isEmpty? I think it will do both of these checks for you. - Reuben Kuhnert On 六月 6, 2016, 6:19 p.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48233/ > ----------------------------------------------------------- > > (Updated 六月 6, 2016, 6:19 p.m.) > > > Review request for hive, Mohit Sabharwal and Naveen Gangam. > > > Bugs: HIVE-13884 > https://issues.apache.org/jira/browse/HIVE-13884 > > > Repository: hive-git > > > Description > ------- > > The patch verifies the # of partitions a table has before fetching any from > the metastore. I > t checks that limit from 'hive.limit.query.max.table.partition'. > > A limitation added here is that the variable must be on hive-site.xml in > order to work, and it does not accept to set this through beeline because > HiveMetaStore.java does not read the variables set through beeline. I think > it is better to keep it this way to avoid users changing the value on fly, > and crashing the metastore. > > Another change is that EXPLAIN commands won't be executed either. EXPLAIN > commands need to fetch partitions in order to create the operator tree. If we > allow EXPLAIN to do that, then we may have the same OOM situations for large > partitions. > > > Diffs > ----- > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 94dd72e6624d13d2503f68d2fd2d2a84859a4500 > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > 8e0bba60cc73890c1566e0f5df965f0f0bcfe0ec > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > b6d5276e49356f30147cb4f10262a2730ba99566 > metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java > a6d3f5385b33b8a4e31ee20ca5cb8f58c97c8702 > metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java > 31f0d7b89670b8a749bbe8a7ff2b4ff9f059a8e2 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 3152e77c3c7152ac4dbe7e779ce35f28044fe3c9 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 86a243609b23e2ca9bb8849f0da863a95e477d5c > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > c3d903b8cc8197ba8bea17145bec1444ed14eb22 > > Diff: https://reviews.apache.org/r/48233/diff/ > > > Testing > ------- > > Waiting for HiveQA. > > > Thanks, > > Sergio Pena > >
