> On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > > Lines 1646 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075732#file2075732line1646> > > > > does this allow for x.y.z? > > Does this work for sd columns and other list fields?
yes, this works for multi-valued fields, so something like sd.cols.name will work. Only name will be set for all the sd.cols for each partition. I will add more documentation here to make it clearer. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > > Lines 1647 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075732#file2075732line1647> > > > > Should this be optional? > > Is this a regexp or something else? What is the syntax here? What are _ > > or % wildcards? This does not need to be optional, the default requiredness of the field makes sure that thrift will always check if this field is set and on the writers will serialize the field only when its possible (null value is not serialized). Will update the documentation > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > > Lines 1648 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075732#file2075732line1648> > > > > Why not just have optional include and exclude patterns instead of a > > pattern and a boolean? Thats an interesting thought. Do you think it would be useful for clients to have something like PARAM_KEY LIKE ('includepattern') AND NOT LIKE ('excludepattern')? It should not be too difficult to add to have two separate include and exclude paramKeyPattern. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > > Lines 1658 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075732#file2075732line1658> > > > > Do we need catalog name? Good point. I will add it. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 56 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line56> > > > > This shows as an unused import removed it > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 65 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line65> > > > > Unused import? removed it. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 852 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line852> > > > > Does this and other new methods belong here or in MetastoreServerUtils? Its unclear what is the difference between MetaStoreUtils and MetastoreServerUtils? Is MetaStoreUtils going to move to common in the near future? If yes, I can move it to MetastoreServerUtils. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 853 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line853> > > > > It would be good to explain what this method actually does converts > > list of objects to a comma-separated string of stringified objects. Then > > you can explain the intended use case. This method is used only in MetastoreDirectSQL class. Moved it back to that class. It was probably a left over from the many iterations of my code changes. Updated the documentations as suggested. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 950 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line950> > > > > What is this class? This class was originally in HMSHandler and used to grouping Partitions based on a common StorageDescriptor. In order to do that, this class is used to generate the hashKey of partitions so that all the partitions having the same hashcode can be grouped together. The hashCode is overridden such that if the sd.cols, input/outputformat, serializationLib and base location is same, it returns the same hashCode so that all such partitions can be grouped together. I had to move this class from HMSHandler to MetaStoreUtils so that this code could be reused by the new API as well as the previously existing APIs using this grouping logic. I think I can move this to MetaStoreServerUtils, but its unclear what goes into MetaStoreUtils and what goes into MetaStoreServerUtils. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 953 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line953> > > > > Do we need this and other fields? Why can't we just use > > this.sd.outputFormat, etc? I thought it would be better to keep them as fields since they will be used over and over again in the equals and hashCode method. I changed the implementation so that it removes the fields and calculates the hashCode at the object construction time only once. > On Aug. 23, 2018, 6:26 a.m., Alexander Kolbasov wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > > Lines 978 (patched) > > <https://reviews.apache.org/r/68395/diff/3/?file=2075744#file2075744line978> > > > > Why do we need an explicit empty constructor? Yes, its used to represent a unsetSDKey in get_partitionspecs_grouped_by_storage_descriptor which is used when sd is not in the requested list of fields. - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68395/#review207802 ----------------------------------------------------------- On Aug. 23, 2018, 6:44 p.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68395/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2018, 6:44 p.m.) > > > Review request for hive, Alexander Kolbasov, Alan Gates, Peter Vary, and > Sergey Shelukhin. > > > Bugs: HIVE-20306 > https://issues.apache.org/jira/browse/HIVE-20306 > > > Repository: hive-git > > > Description > ------- > > HIVE-20306 : Implement projection spec for fetching only requested fields > from partitions > > > Diffs > ----- > > > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java > 0ad2a2469e0330e050fdb8983078b80617afbbf1 > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java > PRE-CREATION > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectSpec.java > PRE-CREATION > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java > PRE-CREATION > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java > PRE-CREATION > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionFilterMode.java > PRE-CREATION > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java > 7ab64eadac7948a7f5077260694926cc5b6e4e4b > > standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php > cc19f2389e7b595722dcc1f3296877a02b20e0a4 > > standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php > 5ed4f71b1dd947c7d4cbb5c290b393b8cd2ea31d > > standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote > a595732f04af4304974186178377192227bb80fb > > standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py > d098dba100bdfe56aa6ecafb31880098a2d7c6cb > > standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py > 7fc1e43de03eac1cfe802439ba38f83988299169 > > standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb > e0c6c02715dab4d9ad457ec710bcb3159206c6c6 > > standalone-metastore/metastore-common/src/gen/thrift/gen-rb/thrift_hive_metastore.rb > 92424a43feefc8c0db7c91302045437f3afbf274 > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > 8a4bdd8ed827572f5fd9d291c5454630d84284bd > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 324035a8096a001d3fb170b8640805258d5e2cdd > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 5ae00af564b05ec2720a03072f94c8f8579378a7 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > 91405b9a334a4b031a5dc7f4a1757a3895bfb386 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > 571c789eddfd2b1a27c65c48bdc6dccfafaaf676 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDirectSqlUtils.java > PRE-CREATION > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > d27224b23580b4662a85c874b657847ed068c9a3 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/PartitionProjectionEvaluator.java > PRE-CREATION > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java > b61ee81533930c889f23d2551041055cbdd1a6b2 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/StatObjectConverter.java > 7a0b21b2580d8bb9b256dbc698f125ed15ccdcd3 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java > 0445cbf9095285bdcde72946f1b6dd9a9a3b9fff > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/model/MSerDeInfo.java > 68f07e2569b6531cf3e18919209aed1a17e88bf7 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java > 5233bee59220244e89f05b6c4dbf86a2cc6dc9fe > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 0934aeb3a7d5413cacde500a5575e4f676306bd0 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 70a17f51b9b5a9fb0b5640988318fd39a82b895d > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java > 35abd006d41f0eca354123fcfe6f590867f80cac > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestGetPartitionsUsingProjection.java > PRE-CREATION > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java > 60beab6350d1f4d86bdcf79f5119172117c5ca2e > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestPartitionProjectionEvaluator.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/68395/diff/4/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
