You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jesús Camacho Rodríguez <jc...@hortonworks.com> on 2018/03/29 20:56:10 UTC

Review Request 66369: HIVE-18839

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

Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
-------

HIVE-18839


Diffs
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 697b1940a928000d23e414025d990ef50d20ea69 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 01cadeaaab2c8dd97b081d59baa06f450338027f 
  ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java de120afbbcd02dc23dff163c14fd7c8fba25db0c 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 89ca1ff5cf1135cda85090ad2d03088b51680151 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 0db2a2c3ed5fda2352f2f63acbce8c6b4b889b84 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java d76c8b6f40e8ab8a62875f6801d40b6c23c0ae85 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 26f20f2e05478fdbd05e494bf1a843567f2dff2c 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_5.q.out PRE-CREATION 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h e10a655e762eaaa5b0c32b2d1cf43a80a42410bc 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 0f49d93277165e572c9b607eeed9ab3a168c1c0b 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp d7319e247524d82603bc27d1bb791cbe1c79d740 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 724f02229b54f0e9a7ec8ab0546a83650860d1f1 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 8e357f6422fecfecac5a558c04ec741f06b87a62 
  standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java fec35d50b7009439da7f7b40911e4816996fb4fc 
  standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java e824f4a145cd049eb712b372dbf7fea3b7c36d42 
  standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php d00d11be3e054b805cb5c09c0222da2aa3a013f7 
  standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php 32c8cb7bc57eb09a67bb971bcfe49155f6790a78 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1b3ebcf6ef9775785008bd3b549516f8932dd64f 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py cf36654b5127a445577197dbf365352081e64e96 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py fd157b2781f0746234ab62548855f14b5fc4ed64 
  standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb aa93158fa621e48552c0d108b68a12d8ed3589e9 
  standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 7a07b73cc709684e8c548ebbbe4882d562f069e5 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8a5de09ccf1ebc21a86f479a65bb6e78defc3c31 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java d4bdcf1a79b18e61eb5d427646019a6dbcc9b4c0 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java c69192b73195dcfae3f2567d419ef0b0cf913a1e 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 1636d48d2c7dd21ea25dd3d670103ee3c618df38 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java b8976ed953d1c449ce7d78b3cd37b39218e42bc2 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java e453e5a7d2ec75a962194ae95a866aafee6e1b47 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java 38fa0e2daa17fddda139087d20bc43573af0ea9c 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 68d7b45d0dae8bc8509a672c39dd57b029433e3c 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 6d3f68c0f0723dcdf15ee80de91a6c5a18c202ef 


Diff: https://reviews.apache.org/r/66369/diff/1/


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 66369: HIVE-18839

Posted by Ashutosh Chauhan <ha...@apache.org>.

> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1460 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990388#file1990388line1471>
> >
> >     This will update all MV before they are considered for rewrite. Looks like we are doing extra work which could be avoided by delaying augmenting MV until it qualifies for rewrite.
> 
> Jesús Camacho Rodríguez wrote:
>     Rewriting algorithm needs those filters to produce correct rewriting, hence though we are indeed doing extra work, I do not think we can avoid it. This part is also useful when MVs are outdated (not only for rebuild), as they may still be used to produce correct rewritings.
>     
>     On the other hand, we could avoid this work by implementing the pre-filtering techniques that we were talking about at some point. The idea would be that only views relevant for a given query will be returned by the method, then they would be enriched.
>     
>     Another improvement is that for a rebuild operation, we only consider in the rewriting the MV that we are rebuilding? However, this is a kind of heuristic, since other MVs that are up-to-date may be useful to rewrite the rebuild query.

Yeah, this makes pre-filtering even more important. I thought we had prefiltering based on table and col names. If not, we should add that in follow-up.
Heuristic for rebuidling makes lot of sense. Since I dont expect overlapping MVs to be common. Heuristic will also improve compile time.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
> > Lines 57-60 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990390#file1990390line57>
> >
> >     Is this rewrite always better?
> >     
> >     Imagine tabA and tabB to be in millions rows but their join generates cross product of billions of rows. In prev case we were doing union of large MV with join of two tables (with which filter may get very small) as compared to here where we do ROJ of MV with one source table.
> 
> Jesús Camacho Rodríguez wrote:
>     Comment may be misleading. In the last MERGE, _source_ refers to _source_ block, not to a single query:
>     _(SELECT a, b, SUM(x) AS s, COUNT(*) AS c --NEW DATA
>      FROM TAB_A
>      JOIN TAB_B ON (TAB_A.a = TAB_B.z)
>      WHERE TAB_A.ROW_ID > 5
>      GROUP BY a, b) source_
>     
>     In any case, I think the point about performance is relevant. I expect if we do a rebuild with not many new rows, incremental maintenance will get the edge. However, if there are many changes in source tables, union may be benefitial over ROJ? But in the latter case, we also need to take into account that the UNION operation will invalidate the LLAP cache of the MV, while the ROJ variant will not. I can add a flag to control the incremental maintenance piece, even if it is always true, in case we want to disable it to do some tests in the future? What do you think?

yeah.. lets add a flag with default to true.


- Ashutosh


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


On April 5, 2018, 2:23 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66369/
> -----------------------------------------------------------
> 
> (Updated April 5, 2018, 2:23 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18839
>     https://issues.apache.org/jira/browse/HIVE-18839
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-18839
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5f07312651dc64fe23dcb21d185d6676f33f3bb0 
>   itests/src/test/resources/testconfiguration.properties d2e077b5096cbf82f7b8a98e8f66a5ef98e72bc9 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java fb1efe01dcddb4fd77f05918e46d742922dd313d 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java 50fc4e0c63a060781c93f0e65c5da32dd9116d70 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 5a95649f5bccbdd44cb4c4d12c65d39a5a733a56 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 490f3b8ffe7564d10d86293b402d3c0dad2f7ee7 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java eed37a19376c398cacd9f8e86551591eed89c07c 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 99e2c72d212cfaefe463b1fa32239d6b63aa9228 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_5.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 18eb1d1daa483d4635db514beb96104b558ee1bd 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_2.q.out 144f9d84208ab32528a20bc6151c9be58571291e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out 32b408f5e4d389f9bd143695f8dd570db234a876 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_time_window.q.out bfbac3f3492fdd15b55475cd423cf262793cff84 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 75e828fd4296d5f9333356916ba785ca664bfcb6 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 6e8b2e3e6593b9e921517cb349755a7b668dc900 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out e3bd233177a9058cb202e641175a5fbb4aa7ca89 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 7301571cbf06b874fa027aa8e7a9bc1192a5c948 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out bab3cacc2ce420578732037fb2cefc3d375c4544 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 017d793283904ca83fc1074eb01ca9879805a61a 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 486a50d55a346b7c2dd1e4e30a70ac7663da1d5d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 1ca06d3cc4207818f059650b0bc250433d41c783 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 3120e0db7a19017d9edecc541f5bf1a1d444e66d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb.q.out bc42df022734c76c18dc32c64509dd16addd1d3f 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb_2.q.out d561208fd8345bb338c0020ce6449fffd6067507 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h b9e8e24b6356feea2abd0777a1bd41907b6a4afc 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp c24055af98878a9422099e9e786252680243fcd5 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp cfec64f96a005dc6a342d267563f384108bfed22 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 086cfe9db6cd64aa6720f63925ee501192e6008f 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 383cb9bc9c1cb06cea4ed4577bcfe1bd81f69c54 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java 69607280bb6fc92596db90c01ea2508fc2fe4cee 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 0bca4ebafd827bc1dd59b05f926bbeab97efb87f 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 5e3dff1a12a03bb5b362ba684e199f5afab589e4 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php d4fcc888ed0ef679fa70ccc30c7d38fa932d5d8a 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote d39690f31ca62181cb535d95cc90203cfaec33d2 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py f8ffeac6053a894fb039c0fad3663f52249406d6 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 972db1f0a39c8178b604e4d30447ee687243b5b3 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 94454a1499e8354ef121074d1038a2f074a14cf5 
>   standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb c1036755b4421bfbcd4c21a036cc8bad5a8e32d8 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8539fea42fa2743381833ab3137579caeac64672 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ebbf465244885f8648800b0f98ec3abad9194fe1 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b2c40c25f25c8069be9da198cc8f8c2dce09c14d 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 80cb1de75e44efebd23e300c991b6715ad15faff 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java 995137f9675cd34b7d7c6f79bb78e206a65d99f7 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 2b815fc2e199914dbbefd884f78e97b23fd4c880 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java e72d3278928e43f479de38f424c36298bb53e4da 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439885f2437359db591802e8c37aae488226 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 7d372627a4a0eb96c739ba7ecf30e2ab5799efb5 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 7a871e14585b0cecd2e101609390e0b9c3672c8e 
> 
> 
> Diff: https://reviews.apache.org/r/66369/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 66369: HIVE-18839

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
> > Line 842 (original)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990385#file1990385line850>
> >
> >     Should this instead be (queryPlan.hasAcidResourcesInQuery() || (queryPlan.isMVRebuild())
> >      or similar ?

This is not part of this patch, it was coming from patch used to mv the id list generation. I have removed this.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
> > Lines 966 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990385#file1990385line972>
> >
> >     Unused function.

It is used in L1115.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
> > Lines 1083 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990385#file1990385line1089>
> >
> >     LOG.warn()
> >     Also add in message that rebuild will fail now.

This may also happen when we succeed, I have just transform it into debug.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java
> > Lines 221 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990387#file1990387line221>
> >
> >     Can you add a comment on why it never actually locks.

// This is default implementation. Locking only works for incremental maintenance
// which only works for DB transactional manager, thus we cannot acquire a lock.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1352-1353 (original), 1376-1377 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990388#file1990388line1384>
> >
> >     Comment doesn't match code. 
> >     Should this be outdated = false?

Comment was outdated indeed, I have removed it. There is a comment just above that explains the logic. _outdated = true_ when _diff == 0L or we are rebuilding the MV_, and _the MV has been invalidated_, because in those cases it cannot be outdated at all.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1460 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990388#file1990388line1471>
> >
> >     This will update all MV before they are considered for rewrite. Looks like we are doing extra work which could be avoided by delaying augmenting MV until it qualifies for rewrite.

Rewriting algorithm needs those filters to produce correct rewriting, hence though we are indeed doing extra work, I do not think we can avoid it. This part is also useful when MVs are outdated (not only for rebuild), as they may still be used to produce correct rewritings.

On the other hand, we could avoid this work by implementing the pre-filtering techniques that we were talking about at some point. The idea would be that only views relevant for a given query will be returned by the method, then they would be enriched.

Another improvement is that for a rebuild operation, we only consider in the rewriting the MV that we are rebuilding? However, this is a kind of heuristic, since other MVs that are up-to-date may be useful to rewrite the rebuild query.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1522 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990388#file1990388line1533>
> >
> >     Is this guaranteed to be an acid table. Else, this may throw NPE when row_id is not present.

This should be an ACID table. _augment_ should not be called otherwise. I have added additional check on the invalidation information so we do not call the method.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
> > Lines 57-60 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990390#file1990390line57>
> >
> >     Is this rewrite always better?
> >     
> >     Imagine tabA and tabB to be in millions rows but their join generates cross product of billions of rows. In prev case we were doing union of large MV with join of two tables (with which filter may get very small) as compared to here where we do ROJ of MV with one source table.

Comment may be misleading. In the last MERGE, _source_ refers to _source_ block, not to a single query:
_(SELECT a, b, SUM(x) AS s, COUNT(*) AS c --NEW DATA
 FROM TAB_A
 JOIN TAB_B ON (TAB_A.a = TAB_B.z)
 WHERE TAB_A.ROW_ID > 5
 GROUP BY a, b) source_

In any case, I think the point about performance is relevant. I expect if we do a rebuild with not many new rows, incremental maintenance will get the edge. However, if there are many changes in source tables, union may be benefitial over ROJ? But in the latter case, we also need to take into account that the UNION operation will invalidate the LLAP cache of the MV, while the ROJ variant will not. I can add a flag to control the incremental maintenance piece, even if it is always true, in case we want to disable it to do some tests in the future? What do you think?


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java
> > Lines 75 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990392#file1990392line75>
> >
> >     should this check inner join ?

Currently rewritings with outer join are not produced, hence check is not needed unless I am missing something?


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990392#file1990392line79>
> >
> >     Doesnt it need to check agg functions which are supported : min, max, count, sum ?

If aggregation function is not supported, we cannot generate the rewriting. Supported functions are identified in _HiveRelBuilder.getRollup_.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
> > Lines 703-706 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990424#file1990424line703>
> >
> >     These objects are not always needed. If so, we can delay getting them until needed.

_materializationsRebuildLockHandler_ is needed by the _try_ and _finally_ clauses, hence I would not move it (we would need to call method twice).
_materializationsInvalidationCache_ is only needed in the _try_ clause, I have moved it.


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > standalone-metastore/src/main/thrift/hive_metastore.thrift
> > Lines 1164-1165 (original), 1164-1166 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990426#file1990426line1164>
> >
> >     Shouldn't they be required?

These two fields may not be set when Materialization is created. Then, when it is retrieved, they are populated with invalidation time and whether there was a source table update/delete when we retrieve them from the invalidation cache.


- Jesús


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


On March 29, 2018, 8:56 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66369/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 8:56 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18839
>     https://issues.apache.org/jira/browse/HIVE-18839
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-18839
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 697b1940a928000d23e414025d990ef50d20ea69 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 01cadeaaab2c8dd97b081d59baa06f450338027f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java de120afbbcd02dc23dff163c14fd7c8fba25db0c 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 89ca1ff5cf1135cda85090ad2d03088b51680151 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 0db2a2c3ed5fda2352f2f63acbce8c6b4b889b84 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java d76c8b6f40e8ab8a62875f6801d40b6c23c0ae85 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 26f20f2e05478fdbd05e494bf1a843567f2dff2c 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_5.q.out PRE-CREATION 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h e10a655e762eaaa5b0c32b2d1cf43a80a42410bc 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 0f49d93277165e572c9b607eeed9ab3a168c1c0b 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp d7319e247524d82603bc27d1bb791cbe1c79d740 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 724f02229b54f0e9a7ec8ab0546a83650860d1f1 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 8e357f6422fecfecac5a558c04ec741f06b87a62 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java fec35d50b7009439da7f7b40911e4816996fb4fc 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java e824f4a145cd049eb712b372dbf7fea3b7c36d42 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php d00d11be3e054b805cb5c09c0222da2aa3a013f7 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php 32c8cb7bc57eb09a67bb971bcfe49155f6790a78 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1b3ebcf6ef9775785008bd3b549516f8932dd64f 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py cf36654b5127a445577197dbf365352081e64e96 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py fd157b2781f0746234ab62548855f14b5fc4ed64 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb aa93158fa621e48552c0d108b68a12d8ed3589e9 
>   standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 7a07b73cc709684e8c548ebbbe4882d562f069e5 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8a5de09ccf1ebc21a86f479a65bb6e78defc3c31 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java d4bdcf1a79b18e61eb5d427646019a6dbcc9b4c0 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java c69192b73195dcfae3f2567d419ef0b0cf913a1e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 1636d48d2c7dd21ea25dd3d670103ee3c618df38 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java b8976ed953d1c449ce7d78b3cd37b39218e42bc2 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java e453e5a7d2ec75a962194ae95a866aafee6e1b47 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java 38fa0e2daa17fddda139087d20bc43573af0ea9c 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 68d7b45d0dae8bc8509a672c39dd57b029433e3c 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 6d3f68c0f0723dcdf15ee80de91a6c5a18c202ef 
> 
> 
> Diff: https://reviews.apache.org/r/66369/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 66369: HIVE-18839

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1460 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990388#file1990388line1471>
> >
> >     This will update all MV before they are considered for rewrite. Looks like we are doing extra work which could be avoided by delaying augmenting MV until it qualifies for rewrite.
> 
> Jesús Camacho Rodríguez wrote:
>     Rewriting algorithm needs those filters to produce correct rewriting, hence though we are indeed doing extra work, I do not think we can avoid it. This part is also useful when MVs are outdated (not only for rebuild), as they may still be used to produce correct rewritings.
>     
>     On the other hand, we could avoid this work by implementing the pre-filtering techniques that we were talking about at some point. The idea would be that only views relevant for a given query will be returned by the method, then they would be enriched.
>     
>     Another improvement is that for a rebuild operation, we only consider in the rewriting the MV that we are rebuilding? However, this is a kind of heuristic, since other MVs that are up-to-date may be useful to rewrite the rebuild query.
> 
> Ashutosh Chauhan wrote:
>     Yeah, this makes pre-filtering even more important. I thought we had prefiltering based on table and col names. If not, we should add that in follow-up.
>     Heuristic for rebuidling makes lot of sense. Since I dont expect overlapping MVs to be common. Heuristic will also improve compile time.

Prefiltering based on tables in present in Calcite, but we need to add the materializations to the planner first. Here logic is a bit different, because we are enriching the materializations before adding them to the planner. In any case, for the rebuild, which is the most common case of incremental rewriting, adding the heuristic will give us a boost. I just did it and added comments explaining it.

FWIW, in the creation metadata we have information about the tables used by a materialization, so we could do a simple prefilter before loading the materializations in the planner (follow-up).


> On April 3, 2018, 11:34 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
> > Lines 57-60 (patched)
> > <https://reviews.apache.org/r/66369/diff/1/?file=1990390#file1990390line57>
> >
> >     Is this rewrite always better?
> >     
> >     Imagine tabA and tabB to be in millions rows but their join generates cross product of billions of rows. In prev case we were doing union of large MV with join of two tables (with which filter may get very small) as compared to here where we do ROJ of MV with one source table.
> 
> Jesús Camacho Rodríguez wrote:
>     Comment may be misleading. In the last MERGE, _source_ refers to _source_ block, not to a single query:
>     _(SELECT a, b, SUM(x) AS s, COUNT(*) AS c --NEW DATA
>      FROM TAB_A
>      JOIN TAB_B ON (TAB_A.a = TAB_B.z)
>      WHERE TAB_A.ROW_ID > 5
>      GROUP BY a, b) source_
>     
>     In any case, I think the point about performance is relevant. I expect if we do a rebuild with not many new rows, incremental maintenance will get the edge. However, if there are many changes in source tables, union may be benefitial over ROJ? But in the latter case, we also need to take into account that the UNION operation will invalidate the LLAP cache of the MV, while the ROJ variant will not. I can add a flag to control the incremental maintenance piece, even if it is always true, in case we want to disable it to do some tests in the future? What do you think?
> 
> Ashutosh Chauhan wrote:
>     yeah.. lets add a flag with default to true.

Added hive.materializedview.rebuild.incremental.


- Jesús


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


On April 5, 2018, 2:23 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66369/
> -----------------------------------------------------------
> 
> (Updated April 5, 2018, 2:23 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18839
>     https://issues.apache.org/jira/browse/HIVE-18839
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-18839
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5f07312651dc64fe23dcb21d185d6676f33f3bb0 
>   itests/src/test/resources/testconfiguration.properties d2e077b5096cbf82f7b8a98e8f66a5ef98e72bc9 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java fb1efe01dcddb4fd77f05918e46d742922dd313d 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java 50fc4e0c63a060781c93f0e65c5da32dd9116d70 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 5a95649f5bccbdd44cb4c4d12c65d39a5a733a56 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 490f3b8ffe7564d10d86293b402d3c0dad2f7ee7 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java eed37a19376c398cacd9f8e86551591eed89c07c 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 99e2c72d212cfaefe463b1fa32239d6b63aa9228 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_5.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 18eb1d1daa483d4635db514beb96104b558ee1bd 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_2.q.out 144f9d84208ab32528a20bc6151c9be58571291e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out 32b408f5e4d389f9bd143695f8dd570db234a876 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_time_window.q.out bfbac3f3492fdd15b55475cd423cf262793cff84 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 75e828fd4296d5f9333356916ba785ca664bfcb6 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 6e8b2e3e6593b9e921517cb349755a7b668dc900 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out e3bd233177a9058cb202e641175a5fbb4aa7ca89 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 7301571cbf06b874fa027aa8e7a9bc1192a5c948 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out bab3cacc2ce420578732037fb2cefc3d375c4544 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 017d793283904ca83fc1074eb01ca9879805a61a 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 486a50d55a346b7c2dd1e4e30a70ac7663da1d5d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 1ca06d3cc4207818f059650b0bc250433d41c783 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 3120e0db7a19017d9edecc541f5bf1a1d444e66d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb.q.out bc42df022734c76c18dc32c64509dd16addd1d3f 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb_2.q.out d561208fd8345bb338c0020ce6449fffd6067507 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h b9e8e24b6356feea2abd0777a1bd41907b6a4afc 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp c24055af98878a9422099e9e786252680243fcd5 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp cfec64f96a005dc6a342d267563f384108bfed22 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 086cfe9db6cd64aa6720f63925ee501192e6008f 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 383cb9bc9c1cb06cea4ed4577bcfe1bd81f69c54 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java 69607280bb6fc92596db90c01ea2508fc2fe4cee 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 0bca4ebafd827bc1dd59b05f926bbeab97efb87f 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 5e3dff1a12a03bb5b362ba684e199f5afab589e4 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php d4fcc888ed0ef679fa70ccc30c7d38fa932d5d8a 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote d39690f31ca62181cb535d95cc90203cfaec33d2 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py f8ffeac6053a894fb039c0fad3663f52249406d6 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 972db1f0a39c8178b604e4d30447ee687243b5b3 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 94454a1499e8354ef121074d1038a2f074a14cf5 
>   standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb c1036755b4421bfbcd4c21a036cc8bad5a8e32d8 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8539fea42fa2743381833ab3137579caeac64672 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ebbf465244885f8648800b0f98ec3abad9194fe1 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b2c40c25f25c8069be9da198cc8f8c2dce09c14d 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 80cb1de75e44efebd23e300c991b6715ad15faff 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java 995137f9675cd34b7d7c6f79bb78e206a65d99f7 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 2b815fc2e199914dbbefd884f78e97b23fd4c880 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java e72d3278928e43f479de38f424c36298bb53e4da 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439885f2437359db591802e8c37aae488226 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 7d372627a4a0eb96c739ba7ecf30e2ab5799efb5 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 7a871e14585b0cecd2e101609390e0b9c3672c8e 
> 
> 
> Diff: https://reviews.apache.org/r/66369/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 66369: HIVE-18839

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66369/#review200335
-----------------------------------------------------------




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Line 1955 (original), 1959 (patched)
<https://reviews.apache.org/r/66369/#comment281040>

    accidental change?



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
Lines 719 (patched)
<https://reviews.apache.org/r/66369/#comment281041>

    LOG.debug



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
Line 842 (original)
<https://reviews.apache.org/r/66369/#comment281042>

    Should this instead be (queryPlan.hasAcidResourcesInQuery() || (queryPlan.isMVRebuild())
     or similar ?



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
Lines 960 (patched)
<https://reviews.apache.org/r/66369/#comment281044>

    LOG.debug()



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
Lines 966 (patched)
<https://reviews.apache.org/r/66369/#comment281045>

    Unused function.



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
Lines 1083 (patched)
<https://reviews.apache.org/r/66369/#comment281046>

    LOG.warn()
    Also add in message that rebuild will fail now.



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java
Lines 256 (patched)
<https://reviews.apache.org/r/66369/#comment281047>

    Is arg dbName.TblName? If so, it should instead be dbName, tblName



ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java
Lines 221 (patched)
<https://reviews.apache.org/r/66369/#comment281048>

    Can you add a comment on why it never actually locks.



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1358 (patched)
<https://reviews.apache.org/r/66369/#comment281049>

    should this be diff < 0 ?



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1352-1353 (original), 1376-1377 (patched)
<https://reviews.apache.org/r/66369/#comment281081>

    Comment doesn't match code. 
    Should this be outdated = false?



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1460 (patched)
<https://reviews.apache.org/r/66369/#comment281125>

    This will update all MV before they are considered for rewrite. Looks like we are doing extra work which could be avoided by delaying augmenting MV until it qualifies for rewrite.



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1483 (patched)
<https://reviews.apache.org/r/66369/#comment281082>

    It will be good to expand on how rewriting uses this info.



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1522 (patched)
<https://reviews.apache.org/r/66369/#comment281083>

    Is this guaranteed to be an acid table. Else, this may throw NPE when row_id is not present.



ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
Lines 196 (patched)
<https://reviews.apache.org/r/66369/#comment281050>

    LOG.debug



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
Lines 57-60 (patched)
<https://reviews.apache.org/r/66369/#comment281137>

    Is this rewrite always better?
    
    Imagine tabA and tabB to be in millions rows but their join generates cross product of billions of rows. In prev case we were doing union of large MV with join of two tables (with which filter may get very small) as compared to here where we do ROJ of MV with one source table.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java
Lines 75 (patched)
<https://reviews.apache.org/r/66369/#comment281140>

    should this check inner join ?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java
Lines 79 (patched)
<https://reviews.apache.org/r/66369/#comment281141>

    Doesnt it need to check agg functions which are supported : min, max, count, sum ?



ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java
Lines 100 (patched)
<https://reviews.apache.org/r/66369/#comment281146>

    LOG.debug



ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out
Line 443 (original), 443 (patched)
<https://reviews.apache.org/r/66369/#comment281051>

    Lets move and keep all tests only on llap local. Testing on MR is not useful



ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out
Line 73 (original), 73 (patched)
<https://reviews.apache.org/r/66369/#comment281147>

    Migrate test to run only on minillaplocal



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
Lines 2018 (patched)
<https://reviews.apache.org/r/66369/#comment281052>

    We should instead pass dbName and tblName seperately.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
Lines 2026 (patched)
<https://reviews.apache.org/r/66369/#comment281053>

    Dbname, tblName.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java
Lines 57 (patched)
<https://reviews.apache.org/r/66369/#comment281148>

    LOG.info



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Line 804 (original), 806 (patched)
<https://reviews.apache.org/r/66369/#comment281150>

    accidental change?



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
Lines 703-706 (patched)
<https://reviews.apache.org/r/66369/#comment281152>

    These objects are not always needed. If so, we can delay getting them until needed.



standalone-metastore/src/main/thrift/hive_metastore.thrift
Lines 1164-1165 (original), 1164-1166 (patched)
<https://reviews.apache.org/r/66369/#comment281151>

    Shouldn't they be required?



standalone-metastore/src/main/thrift/hive_metastore.thrift
Lines 2095-2096 (patched)
<https://reviews.apache.org/r/66369/#comment281080>

    Api should have dbname, tbl name.


- Ashutosh Chauhan


On March 29, 2018, 8:56 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66369/
> -----------------------------------------------------------
> 
> (Updated March 29, 2018, 8:56 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18839
>     https://issues.apache.org/jira/browse/HIVE-18839
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-18839
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 697b1940a928000d23e414025d990ef50d20ea69 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 01cadeaaab2c8dd97b081d59baa06f450338027f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java de120afbbcd02dc23dff163c14fd7c8fba25db0c 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 89ca1ff5cf1135cda85090ad2d03088b51680151 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 0db2a2c3ed5fda2352f2f63acbce8c6b4b889b84 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java d76c8b6f40e8ab8a62875f6801d40b6c23c0ae85 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 26f20f2e05478fdbd05e494bf1a843567f2dff2c 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_5.q.out PRE-CREATION 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h e10a655e762eaaa5b0c32b2d1cf43a80a42410bc 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 0f49d93277165e572c9b607eeed9ab3a168c1c0b 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp d7319e247524d82603bc27d1bb791cbe1c79d740 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 724f02229b54f0e9a7ec8ab0546a83650860d1f1 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 8e357f6422fecfecac5a558c04ec741f06b87a62 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java fec35d50b7009439da7f7b40911e4816996fb4fc 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java e824f4a145cd049eb712b372dbf7fea3b7c36d42 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php d00d11be3e054b805cb5c09c0222da2aa3a013f7 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php 32c8cb7bc57eb09a67bb971bcfe49155f6790a78 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1b3ebcf6ef9775785008bd3b549516f8932dd64f 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py cf36654b5127a445577197dbf365352081e64e96 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py fd157b2781f0746234ab62548855f14b5fc4ed64 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb aa93158fa621e48552c0d108b68a12d8ed3589e9 
>   standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 7a07b73cc709684e8c548ebbbe4882d562f069e5 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8a5de09ccf1ebc21a86f479a65bb6e78defc3c31 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java d4bdcf1a79b18e61eb5d427646019a6dbcc9b4c0 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java c69192b73195dcfae3f2567d419ef0b0cf913a1e 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 1636d48d2c7dd21ea25dd3d670103ee3c618df38 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java b8976ed953d1c449ce7d78b3cd37b39218e42bc2 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java e453e5a7d2ec75a962194ae95a866aafee6e1b47 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java 38fa0e2daa17fddda139087d20bc43573af0ea9c 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 68d7b45d0dae8bc8509a672c39dd57b029433e3c 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 6d3f68c0f0723dcdf15ee80de91a6c5a18c202ef 
> 
> 
> Diff: https://reviews.apache.org/r/66369/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 66369: HIVE-18839

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66369/
-----------------------------------------------------------

(Updated April 9, 2018, 12:15 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
-------

HIVE-18839


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0627c3537840757dae4061e4e729a48bee9d2ec9 
  itests/src/test/resources/testconfiguration.properties 28c14ebc4cede903f19cca79996ccc441774d147 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java fb1efe01dcddb4fd77f05918e46d742922dd313d 
  ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java 50fc4e0c63a060781c93f0e65c5da32dd9116d70 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 5a95649f5bccbdd44cb4c4d12c65d39a5a733a56 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 490f3b8ffe7564d10d86293b402d3c0dad2f7ee7 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 38fbb7ba53d6bae1c0ea8800a97c025ff8e1b8a6 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAugmentMaterializationRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ff0a2e6a1beef08faca3d108c17330af26516934 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_5.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 18eb1d1daa483d4635db514beb96104b558ee1bd 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_2.q.out 144f9d84208ab32528a20bc6151c9be58571291e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out 32b408f5e4d389f9bd143695f8dd570db234a876 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_time_window.q.out bfbac3f3492fdd15b55475cd423cf262793cff84 
  ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 75e828fd4296d5f9333356916ba785ca664bfcb6 
  ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 6e8b2e3e6593b9e921517cb349755a7b668dc900 
  ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out e3bd233177a9058cb202e641175a5fbb4aa7ca89 
  ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 7301571cbf06b874fa027aa8e7a9bc1192a5c948 
  ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out bab3cacc2ce420578732037fb2cefc3d375c4544 
  ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 017d793283904ca83fc1074eb01ca9879805a61a 
  ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 486a50d55a346b7c2dd1e4e30a70ac7663da1d5d 
  ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 1ca06d3cc4207818f059650b0bc250433d41c783 
  ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 3120e0db7a19017d9edecc541f5bf1a1d444e66d 
  ql/src/test/results/clientpositive/materialized_view_rewrite_ssb.q.out bc42df022734c76c18dc32c64509dd16addd1d3f 
  ql/src/test/results/clientpositive/materialized_view_rewrite_ssb_2.q.out d561208fd8345bb338c0020ce6449fffd6067507 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h b9e8e24b6356feea2abd0777a1bd41907b6a4afc 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp c24055af98878a9422099e9e786252680243fcd5 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp cfec64f96a005dc6a342d267563f384108bfed22 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 086cfe9db6cd64aa6720f63925ee501192e6008f 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 383cb9bc9c1cb06cea4ed4577bcfe1bd81f69c54 
  standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java 69607280bb6fc92596db90c01ea2508fc2fe4cee 
  standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 0bca4ebafd827bc1dd59b05f926bbeab97efb87f 
  standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 5e3dff1a12a03bb5b362ba684e199f5afab589e4 
  standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php d4fcc888ed0ef679fa70ccc30c7d38fa932d5d8a 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote d39690f31ca62181cb535d95cc90203cfaec33d2 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py f8ffeac6053a894fb039c0fad3663f52249406d6 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 972db1f0a39c8178b604e4d30447ee687243b5b3 
  standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 94454a1499e8354ef121074d1038a2f074a14cf5 
  standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb c1036755b4421bfbcd4c21a036cc8bad5a8e32d8 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 450da4f0f0491b881200ee1123b8064b023ea3e2 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ebbf465244885f8648800b0f98ec3abad9194fe1 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b2c40c25f25c8069be9da198cc8f8c2dce09c14d 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 80cb1de75e44efebd23e300c991b6715ad15faff 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java 940a1bf276b86dde1c160c0809d701b9ee27930c 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java a79242b1a1c0ad31b75939790cdab9552054e159 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java e72d3278928e43f479de38f424c36298bb53e4da 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439885f2437359db591802e8c37aae488226 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 7d372627a4a0eb96c739ba7ecf30e2ab5799efb5 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 7a871e14585b0cecd2e101609390e0b9c3672c8e 


Diff: https://reviews.apache.org/r/66369/diff/3/

Changes: https://reviews.apache.org/r/66369/diff/2-3/


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 66369: HIVE-18839

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1496 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992649#file1992649line1508>
> >
> >     Lets move this class to  org.apache.hadoop.hive.ql.optimizer.calcite.rules package. It will be easier to find it. Also, it doesn't belong in Hive class.

Moved to org.apache.hadoop.hive.ql.optimizer.calcite.rules.views.


> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 1531 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992649#file1992649line1543>
> >
> >     It will be good to add comment on what filter we construct.
> >     ROW_ID.transactionid <= high_watermark and ROW_ID.transactionid not in (invalid_txns)

Added comment in the rule.


> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
> > Lines 119 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992651#file1992651line119>
> >
> >     Don't we handle count() too ?

We do. When we rollup the COUNT, we will find a SUM on top of the union. Hence, we only need to handle SUM here. Added a comment.


> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 970 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992654#file1992654line980>
> >
> >     It will be good to add why we need to do it this way which is CBO doesn't yet support merge.

Added comment to method.


> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java
> > Lines 66-68 (original), 68-70 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992700#file1992700line70>
> >
> >     Need to update comments.

This info seems up-to-date. I have just added that there is also information about 'whether there was any update or delete in the source tables since the materialized view was created'. Is that what you meant or there is something else missing?


> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java
> > Lines 115 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992702#file1992702line115>
> >
> >     ||  prevResourceLock.state.get() != State.COMMIT_READY

prevResourceLock.state.get() != State.ACQUIRED is correct. If the state is State.ACQUIRED, refresh is valid. But if the state is State.COMMIT_READY, there is no need to refresh anymore.


> On April 7, 2018, 12:29 a.m., Ashutosh Chauhan wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java
> > Lines 135 (patched)
> > <https://reviews.apache.org/r/66369/diff/2/?file=1992702#file1992702line135>
> >
> >     || state != NOT_ACQUIRED

There is no need here to check for NOT_ACQUIRED. When a lock state is NOT_ACQUIRED, the state is not stored in the handler (you can see in L83 that we return the response immediately but we do not store it).


- Jesús


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


On April 5, 2018, 2:23 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66369/
> -----------------------------------------------------------
> 
> (Updated April 5, 2018, 2:23 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18839
>     https://issues.apache.org/jira/browse/HIVE-18839
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-18839
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5f07312651dc64fe23dcb21d185d6676f33f3bb0 
>   itests/src/test/resources/testconfiguration.properties d2e077b5096cbf82f7b8a98e8f66a5ef98e72bc9 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java fb1efe01dcddb4fd77f05918e46d742922dd313d 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java 50fc4e0c63a060781c93f0e65c5da32dd9116d70 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 5a95649f5bccbdd44cb4c4d12c65d39a5a733a56 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 490f3b8ffe7564d10d86293b402d3c0dad2f7ee7 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java eed37a19376c398cacd9f8e86551591eed89c07c 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 99e2c72d212cfaefe463b1fa32239d6b63aa9228 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_5.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 18eb1d1daa483d4635db514beb96104b558ee1bd 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_2.q.out 144f9d84208ab32528a20bc6151c9be58571291e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out 32b408f5e4d389f9bd143695f8dd570db234a876 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_time_window.q.out bfbac3f3492fdd15b55475cd423cf262793cff84 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 75e828fd4296d5f9333356916ba785ca664bfcb6 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 6e8b2e3e6593b9e921517cb349755a7b668dc900 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out e3bd233177a9058cb202e641175a5fbb4aa7ca89 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 7301571cbf06b874fa027aa8e7a9bc1192a5c948 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out bab3cacc2ce420578732037fb2cefc3d375c4544 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 017d793283904ca83fc1074eb01ca9879805a61a 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 486a50d55a346b7c2dd1e4e30a70ac7663da1d5d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 1ca06d3cc4207818f059650b0bc250433d41c783 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 3120e0db7a19017d9edecc541f5bf1a1d444e66d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb.q.out bc42df022734c76c18dc32c64509dd16addd1d3f 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb_2.q.out d561208fd8345bb338c0020ce6449fffd6067507 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h b9e8e24b6356feea2abd0777a1bd41907b6a4afc 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp c24055af98878a9422099e9e786252680243fcd5 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp cfec64f96a005dc6a342d267563f384108bfed22 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 086cfe9db6cd64aa6720f63925ee501192e6008f 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 383cb9bc9c1cb06cea4ed4577bcfe1bd81f69c54 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java 69607280bb6fc92596db90c01ea2508fc2fe4cee 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 0bca4ebafd827bc1dd59b05f926bbeab97efb87f 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 5e3dff1a12a03bb5b362ba684e199f5afab589e4 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php d4fcc888ed0ef679fa70ccc30c7d38fa932d5d8a 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote d39690f31ca62181cb535d95cc90203cfaec33d2 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py f8ffeac6053a894fb039c0fad3663f52249406d6 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 972db1f0a39c8178b604e4d30447ee687243b5b3 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 94454a1499e8354ef121074d1038a2f074a14cf5 
>   standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb c1036755b4421bfbcd4c21a036cc8bad5a8e32d8 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8539fea42fa2743381833ab3137579caeac64672 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ebbf465244885f8648800b0f98ec3abad9194fe1 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b2c40c25f25c8069be9da198cc8f8c2dce09c14d 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 80cb1de75e44efebd23e300c991b6715ad15faff 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java 995137f9675cd34b7d7c6f79bb78e206a65d99f7 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 2b815fc2e199914dbbefd884f78e97b23fd4c880 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java e72d3278928e43f479de38f424c36298bb53e4da 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439885f2437359db591802e8c37aae488226 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 7d372627a4a0eb96c739ba7ecf30e2ab5799efb5 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 7a871e14585b0cecd2e101609390e0b9c3672c8e 
> 
> 
> Diff: https://reviews.apache.org/r/66369/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 66369: HIVE-18839

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66369/#review200681
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java
Lines 1113 (patched)
<https://reviews.apache.org/r/66369/#comment281528>

    LOG.trace()



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1496 (patched)
<https://reviews.apache.org/r/66369/#comment281531>

    Lets move this class to  org.apache.hadoop.hive.ql.optimizer.calcite.rules package. It will be easier to find it. Also, it doesn't belong in Hive class.



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 1531 (patched)
<https://reviews.apache.org/r/66369/#comment281530>

    It will be good to add comment on what filter we construct.
    ROW_ID.transactionid <= high_watermark and ROW_ID.transactionid not in (invalid_txns)



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java
Lines 119 (patched)
<https://reviews.apache.org/r/66369/#comment281532>

    Don't we handle count() too ?



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 970 (patched)
<https://reviews.apache.org/r/66369/#comment281538>

    It will be good to add why we need to do it this way which is CBO doesn't yet support merge.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java
Lines 66-68 (original), 68-70 (patched)
<https://reviews.apache.org/r/66369/#comment281542>

    Need to update comments.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java
Lines 115 (patched)
<https://reviews.apache.org/r/66369/#comment281540>

    ||  prevResourceLock.state.get() != State.COMMIT_READY



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java
Lines 135 (patched)
<https://reviews.apache.org/r/66369/#comment281541>

    || state != NOT_ACQUIRED


- Ashutosh Chauhan


On April 5, 2018, 2:23 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66369/
> -----------------------------------------------------------
> 
> (Updated April 5, 2018, 2:23 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-18839
>     https://issues.apache.org/jira/browse/HIVE-18839
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-18839
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5f07312651dc64fe23dcb21d185d6676f33f3bb0 
>   itests/src/test/resources/testconfiguration.properties d2e077b5096cbf82f7b8a98e8f66a5ef98e72bc9 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java fb1efe01dcddb4fd77f05918e46d742922dd313d 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java 50fc4e0c63a060781c93f0e65c5da32dd9116d70 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 5a95649f5bccbdd44cb4c4d12c65d39a5a733a56 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 490f3b8ffe7564d10d86293b402d3c0dad2f7ee7 
>   ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java eed37a19376c398cacd9f8e86551591eed89c07c 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 99e2c72d212cfaefe463b1fa32239d6b63aa9228 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_5.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
>   ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 18eb1d1daa483d4635db514beb96104b558ee1bd 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_2.q.out 144f9d84208ab32528a20bc6151c9be58571291e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out 32b408f5e4d389f9bd143695f8dd570db234a876 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite_time_window.q.out bfbac3f3492fdd15b55475cd423cf262793cff84 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 75e828fd4296d5f9333356916ba785ca664bfcb6 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 6e8b2e3e6593b9e921517cb349755a7b668dc900 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out e3bd233177a9058cb202e641175a5fbb4aa7ca89 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 7301571cbf06b874fa027aa8e7a9bc1192a5c948 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out bab3cacc2ce420578732037fb2cefc3d375c4544 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 017d793283904ca83fc1074eb01ca9879805a61a 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 486a50d55a346b7c2dd1e4e30a70ac7663da1d5d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 1ca06d3cc4207818f059650b0bc250433d41c783 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 3120e0db7a19017d9edecc541f5bf1a1d444e66d 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb.q.out bc42df022734c76c18dc32c64509dd16addd1d3f 
>   ql/src/test/results/clientpositive/materialized_view_rewrite_ssb_2.q.out d561208fd8345bb338c0020ce6449fffd6067507 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h b9e8e24b6356feea2abd0777a1bd41907b6a4afc 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp c24055af98878a9422099e9e786252680243fcd5 
>   standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp cfec64f96a005dc6a342d267563f384108bfed22 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 086cfe9db6cd64aa6720f63925ee501192e6008f 
>   standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 383cb9bc9c1cb06cea4ed4577bcfe1bd81f69c54 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java 69607280bb6fc92596db90c01ea2508fc2fe4cee 
>   standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 0bca4ebafd827bc1dd59b05f926bbeab97efb87f 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 5e3dff1a12a03bb5b362ba684e199f5afab589e4 
>   standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php d4fcc888ed0ef679fa70ccc30c7d38fa932d5d8a 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote d39690f31ca62181cb535d95cc90203cfaec33d2 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py f8ffeac6053a894fb039c0fad3663f52249406d6 
>   standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 972db1f0a39c8178b604e4d30447ee687243b5b3 
>   standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 94454a1499e8354ef121074d1038a2f074a14cf5 
>   standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb c1036755b4421bfbcd4c21a036cc8bad5a8e32d8 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8539fea42fa2743381833ab3137579caeac64672 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ebbf465244885f8648800b0f98ec3abad9194fe1 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b2c40c25f25c8069be9da198cc8f8c2dce09c14d 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 80cb1de75e44efebd23e300c991b6715ad15faff 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java 995137f9675cd34b7d7c6f79bb78e206a65d99f7 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 2b815fc2e199914dbbefd884f78e97b23fd4c880 
>   standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java e72d3278928e43f479de38f424c36298bb53e4da 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439885f2437359db591802e8c37aae488226 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 7d372627a4a0eb96c739ba7ecf30e2ab5799efb5 
>   standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 7a871e14585b0cecd2e101609390e0b9c3672c8e 
> 
> 
> Diff: https://reviews.apache.org/r/66369/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 66369: HIVE-18839

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66369/
-----------------------------------------------------------

(Updated April 5, 2018, 2:23 a.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
-------

HIVE-18839


Diffs (updated)
-----

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5f07312651dc64fe23dcb21d185d6676f33f3bb0 
  itests/src/test/resources/testconfiguration.properties d2e077b5096cbf82f7b8a98e8f66a5ef98e72bc9 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java fb1efe01dcddb4fd77f05918e46d742922dd313d 
  ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewTask.java 50fc4e0c63a060781c93f0e65c5da32dd9116d70 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 5a95649f5bccbdd44cb4c4d12c65d39a5a733a56 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 490f3b8ffe7564d10d86293b402d3c0dad2f7ee7 
  ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java c8cafa2a68f897b1034e5bf61e872e044e01c22a 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java eed37a19376c398cacd9f8e86551591eed89c07c 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java 53dc8ec1974dfc095bd5c7601c8d486b4712319e 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveAggregateIncrementalRewritingRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveNoAggregateIncrementalRewritingRule.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewRewritingRelVisitor.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 41de17fd4679009ef6a4fb5a6d976cbc794ce791 
  ql/src/java/org/apache/hadoop/hive/ql/parse/MaterializedViewRebuildSemanticAnalyzer.java 75eb50c5797b312f66e81b5cec23849684e641fc 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 99e2c72d212cfaefe463b1fa32239d6b63aa9228 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 4c86fb89376041b30bc5c90a4e84f2e685d8ff82 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q c7f050be8da71f13463c22df2ccdab21c0e40b6a 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q PRE-CREATION 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_5.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out 1ef7b876d82136e3a7ad9021db8c293a3cc4808a 
  ql/src/test/results/clientpositive/llap/materialized_view_rewrite_7.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 18eb1d1daa483d4635db514beb96104b558ee1bd 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_2.q.out 144f9d84208ab32528a20bc6151c9be58571291e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 65614566c9b738ea0354dc6d206a0ed7e635174e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 48c0ecb23f1657c99ef30ed6963a3af1737f0514 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out 32b408f5e4d389f9bd143695f8dd570db234a876 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_time_window.q.out bfbac3f3492fdd15b55475cd423cf262793cff84 
  ql/src/test/results/clientpositive/materialized_view_rewrite_1.q.out 75e828fd4296d5f9333356916ba785ca664bfcb6 
  ql/src/test/results/clientpositive/materialized_view_rewrite_2.q.out 6e8b2e3e6593b9e921517cb349755a7b668dc900 
  ql/src/test/results/clientpositive/materialized_view_rewrite_3.q.out e3bd233177a9058cb202e641175a5fbb4aa7ca89 
  ql/src/test/results/clientpositive/materialized_view_rewrite_4.q.out 7301571cbf06b874fa027aa8e7a9bc1192a5c948 
  ql/src/test/results/clientpositive/materialized_view_rewrite_5.q.out bab3cacc2ce420578732037fb2cefc3d375c4544 
  ql/src/test/results/clientpositive/materialized_view_rewrite_6.q.out 017d793283904ca83fc1074eb01ca9879805a61a 
  ql/src/test/results/clientpositive/materialized_view_rewrite_7.q.out 486a50d55a346b7c2dd1e4e30a70ac7663da1d5d 
  ql/src/test/results/clientpositive/materialized_view_rewrite_8.q.out 1ca06d3cc4207818f059650b0bc250433d41c783 
  ql/src/test/results/clientpositive/materialized_view_rewrite_9.q.out 3120e0db7a19017d9edecc541f5bf1a1d444e66d 
  ql/src/test/results/clientpositive/materialized_view_rewrite_ssb.q.out bc42df022734c76c18dc32c64509dd16addd1d3f 
  ql/src/test/results/clientpositive/materialized_view_rewrite_ssb_2.q.out d561208fd8345bb338c0020ce6449fffd6067507 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h b9e8e24b6356feea2abd0777a1bd41907b6a4afc 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp c24055af98878a9422099e9e786252680243fcd5 
  standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp cfec64f96a005dc6a342d267563f384108bfed22 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 086cfe9db6cd64aa6720f63925ee501192e6008f 
  standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 383cb9bc9c1cb06cea4ed4577bcfe1bd81f69c54 
  standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java 69607280bb6fc92596db90c01ea2508fc2fe4cee 
  standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 0bca4ebafd827bc1dd59b05f926bbeab97efb87f 
  standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 5e3dff1a12a03bb5b362ba684e199f5afab589e4 
  standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php d4fcc888ed0ef679fa70ccc30c7d38fa932d5d8a 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote d39690f31ca62181cb535d95cc90203cfaec33d2 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py f8ffeac6053a894fb039c0fad3663f52249406d6 
  standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 972db1f0a39c8178b604e4d30447ee687243b5b3 
  standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 94454a1499e8354ef121074d1038a2f074a14cf5 
  standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb c1036755b4421bfbcd4c21a036cc8bad5a8e32d8 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 8539fea42fa2743381833ab3137579caeac64672 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java ebbf465244885f8648800b0f98ec3abad9194fe1 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java b2c40c25f25c8069be9da198cc8f8c2dce09c14d 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationInvalidationInfo.java 3d774071c2ff00e892540364a7ec63e49b8089df 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsInvalidationCache.java 80cb1de75e44efebd23e300c991b6715ad15faff 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockCleanerTask.java PRE-CREATION 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MaterializationsRebuildLockHandler.java PRE-CREATION 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java 995137f9675cd34b7d7c6f79bb78e206a65d99f7 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java 2b815fc2e199914dbbefd884f78e97b23fd4c880 
  standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java e72d3278928e43f479de38f424c36298bb53e4da 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439885f2437359db591802e8c37aae488226 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java 7d372627a4a0eb96c739ba7ecf30e2ab5799efb5 
  standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreMaterializationsCacheCleaner.java 7a871e14585b0cecd2e101609390e0b9c3672c8e 


Diff: https://reviews.apache.org/r/66369/diff/2/

Changes: https://reviews.apache.org/r/66369/diff/1-2/


Testing
-------


Thanks,

Jesús Camacho Rodríguez