----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24176/#review49414 -----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java <https://reviews.apache.org/r/24176/#comment86479> I assume this doesn't aggregate the basic stats like #rows, #files, file_size, raw_data_size from PARTITION_PARAMS. If so can you rename this method to getAggrColStatsForPartitions(). Statistics Annotation may also need aggregated basic stats. metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java <https://reviews.apache.org/r/24176/#comment86480> Same here. API name to be explicitly say we are fetching column stats. getPartitionColStats()? metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java <https://reviews.apache.org/r/24176/#comment86481> Should the client (StatsAnnotation) have a fallback mechanism? If directSQL is not available or turned off? ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java <https://reviews.apache.org/r/24176/#comment86482> Same comment about the API naming. ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java <https://reviews.apache.org/r/24176/#comment86484> I think we should leave the state as PARTIAL in both cases. 1) When some partitions doesn't have column stats 2) When some columns doesn't have stats. And probably have debug log for both so that we can atleast know what we are missing. Though StatsAnnotation doesn't worry about 1) its good to know that we are missing something from explain output. ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java <https://reviews.apache.org/r/24176/#comment86483> Will there be any performance gain having JDO fallback vs client side fallback? ql/src/test/results/clientpositive/annotate_stats_part.q.out <https://reviews.apache.org/r/24176/#comment86485> The change from my above comment should revert this back. - Prasanth_J On Aug. 2, 2014, 12:23 a.m., Ashutosh Chauhan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24176/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2014, 12:23 a.m.) > > > Review request for hive and Prasanth_J. > > > Bugs: HIVE-7587 > https://issues.apache.org/jira/browse/HIVE-7587 > > > Repository: hive-git > > > Description > ------- > > Fetch aggregated stats from MetaStore > > > Diffs > ----- > > metastore/if/hive_metastore.thrift 55f41db > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 3b778ee > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 53165d0 > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp > 957b976 > metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h f352cd5 > metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp a6a40fd > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsRequest.java > 4547970 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsResult.java > 68a4219 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AggrStats.java > PRE-CREATION > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ColumnStatistics.java > 6aecf26 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/DropPartitionsResult.java > a4ae892 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/EnvironmentContext.java > ed464d6 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Function.java > 781281a > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsInfoResponse.java > b782d32 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsResponse.java > d549ce9 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPrincipalsInRoleResponse.java > 3ef6224 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetRoleGrantsForPrincipalResponse.java > 3ddc1ac > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HeartbeatTxnRangeResponse.java > f3e3c07 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HiveObjectRef.java > b22b211 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/LockRequest.java > cdf6f30 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/OpenTxnsResponse.java > 54955c6 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Partition.java > 7d29d09 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsByExprResult.java > 5ea5a1b > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsRequest.java > 80a151a > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsResult.java > 537db47 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrincipalPrivilegeSet.java > 0c9518a > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrivilegeBag.java > 4285ed8 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RequestPartsSpec.java > 2fcb216 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Schema.java > 58e9028 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java > b962e27 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowLocksResponse.java > 1399f8b > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SkewedInfo.java > ab5c0ed > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/StorageDescriptor.java > 813b4f0 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java > 484bd6a > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsRequest.java > ddfcccc > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsResult.java > e37b75c > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java > 1e0cdea > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Type.java > 1882b57 > metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php b74e54e > metastore/src/gen/thrift/gen-php/metastore/Types.php 4d4ab84 > metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote > 6fef2cf > metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py > 2a2e443 > metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py c71b7b7 > metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb e21f662 > metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 58b9c0e > 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/IMetaStoreClient.java > d6e849f > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > a23d122 > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 4f186f4 > metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 2379ce7 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 396eb4e > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 80c3b2b > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java > 9620e62 > ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java 818590a > ql/src/test/results/clientpositive/annotate_stats_part.q.out 043af14 > > Diff: https://reviews.apache.org/r/24176/diff/ > > > Testing > ------- > > Existing annotate_stats* tests. > > > Thanks, > > Ashutosh Chauhan > >
