> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java, line 1349
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581012#file1581012line1349>
> >
> >     you may as well use db.getAllTables(dbName). No need to another 
> > function here.

I replaced it by _getMSC().getTableObjectsByName(dbName, 
getMSC().getAllTables(dbName))_, since I need to return the list of Table 
objects (db.getAllTables(dbName) returns list of String objects).


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java, line 1448
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581012#file1581012line1448>
> >
> >     May want to leave a TODO here to get rid of db filtering here, since 
> > Hive allows you to reference tables from different dbs in single query and 
> > we dont want to restrict that.

You are right. Current implementation of materialialized views for rewriting 
are only taken from current database. I think there is no need for this 
limitation, thus I have extended the logic to consider all databases. I will 
add a new test too.


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java,
> >  lines 91-93
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581013#file1581013line91>
> >
> >     May not want to store mv per db, since cross db queries are allowed by 
> > Hive.

I need to double-check behavior in other systems. However, I think even if we 
define a mv with a cross db query, mv itself belongs to a single database 
(where you should have permissions to create it, etc.).


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java,
> >  line 124
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581013#file1581013line124>
> >
> >     May want to leave a TODO about enhancing metastore apis such that it 
> > returns only materailized views instead of all tables.

Yes, you are right. I will create a follow-up JIRA too.


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java,
> >  line 305
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581013#file1581013line305>
> >
> >     This is strong assumption. May want to document this somewhere that all 
> > varchars are assumed to be dimensions.

I just moved around this code, it is part of the Druid integration code that 
was already there. It is documented in the wiki.


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java,
> >  line 343
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581013#file1581013line343>
> >
> >     Not sure if creationDate is useful. May want to leave a TODO to get rid 
> > of it.

I will add a comment. I think it is good to keep it, as this is useful to 
ensure correctness in case multiple HS2 instances are used. Although currently 
we do not encourage to set up this kind of environment, some users might use 
multiple instances, and then the registry uses this date to be sure it is 
keeping the last version of the definition of the mv.


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java, line 1152
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581025#file1581025line1152>
> >
> >     TODO for need for common interface for DruidQuery and TableScan.

I just moved around this code, it is part of the Druid integration code that 
was already there.


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveFilter.java,
> >  lines 46-49
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581018#file1581018line46>
> >
> >     Is this change necessary?

This change is needed. This method is called from VolcanoPlanner, that is only 
used in Hive for materialized view rewriting. If it is not removed, we end up 
in an infinite loop, as md provider calls computeSelfCost and computeSelfCost 
calls md provider.


> On Dec. 13, 2016, 6:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveSemiJoin.java,
> >  lines 110-113
> > <https://reviews.apache.org/r/53111/diff/2/?file=1581020#file1581020line110>
> >
> >     Is this change necessary?

Idem as above.


- Jesús


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


On Dec. 9, 2016, 5:13 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53111/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2016, 5:13 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-14496 and HIVE-14497
>     https://issues.apache.org/jira/browse/HIVE-14496
>     https://issues.apache.org/jira/browse/HIVE-14497
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-14496
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
> 9064e498087da0d778bbc754bc982c0026822fbb 
>   
> hcatalog/core/src/test/java/org/apache/hive/hcatalog/common/TestHCatUtil.java 
> 102d6d224af73f2799f3991b29e5a8695b80dd99 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  690616dc77b2c6c7062f6af349756894cc69862d 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
>  21d1b46fcbd4f8f10ee447dce9d40dd6b43a2793 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggrStatsCacheIntegration.java
>  51d96dd2ea02030860f4830a18d1f99d7592f11f 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseImport.java
>  21f851ef568bbf834b607498fc8b80d0511d7882 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseSchemaTool.java
>  b131163f6ca66be13c4b30686721f272049ecbb4 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreIntegration.java
>  2cc1373108668b26e7e2a64cd5fff893159528bd 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestStorageDescriptorSharing.java
>  c29e46aaf91786cca267ab9c425fa50058189545 
>   metastore/if/hive_metastore.thrift baab31bb0f44361847224843f905c0417b1670be 
>   metastore/scripts/upgrade/derby/037-HIVE-14496.derby.sql PRE-CREATION 
>   metastore/scripts/upgrade/derby/hive-schema-2.2.0.derby.sql 
> ae980e0899df089fcfabc3a6190ef39444a6c2f5 
>   metastore/scripts/upgrade/derby/upgrade-2.1.0-to-2.2.0.derby.sql 
> 25a5e37c2708454388d1671c71a3474be3de4720 
>   metastore/scripts/upgrade/mssql/022-HIVE-14496.mssql.sql PRE-CREATION 
>   metastore/scripts/upgrade/mssql/hive-schema-2.2.0.mssql.sql 
> fdb40048cd26d29ef6afbc8966da0eb3c08b8ac1 
>   metastore/scripts/upgrade/mssql/upgrade-2.1.0-to-2.2.0.mssql.sql 
> df972065ddef1d080872fef003d1b8665ba3f8a2 
>   metastore/scripts/upgrade/mysql/037-HIVE-14496.mysql.sql PRE-CREATION 
>   metastore/scripts/upgrade/mysql/hive-schema-2.2.0.mysql.sql 
> 91e221d8db06a098610b8c39cb664cb8d821e3d8 
>   metastore/scripts/upgrade/mysql/upgrade-2.1.0-to-2.2.0.mysql.sql 
> de38b58dbe0888f9daec0860c9d7ad055a5c5313 
>   metastore/scripts/upgrade/oracle/037-HIVE-14496.oracle.sql PRE-CREATION 
>   metastore/scripts/upgrade/oracle/hive-schema-2.2.0.oracle.sql 
> 39ba7cb3b5e8a68d6d2b543396306c7754a58070 
>   metastore/scripts/upgrade/oracle/upgrade-2.1.0-to-2.2.0.oracle.sql 
> 66784a4e0ec288364c9d44bf72261b6659b9d1a5 
>   metastore/scripts/upgrade/postgres/036-HIVE-14496.postgres.sql PRE-CREATION 
>   metastore/scripts/upgrade/postgres/hive-schema-2.2.0.postgres.sql 
> 63ac3befc2b465fd3d7e9c12a071d81c60dca86b 
>   metastore/scripts/upgrade/postgres/upgrade-2.1.0-to-2.2.0.postgres.sql 
> 0b4591d5aabf25820c17bbe9ae625846e87f7265 
>   
> metastore/src/gen/protobuf/gen-java/org/apache/hadoop/hive/metastore/hbase/HbaseMetastoreProto.java
>  b15b0de5ff81f5a7863c23ec3b198c708e1a4c47 
>   metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 
> 6838133083684ee3b93a93129bb492ab29a4842e 
>   metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 
> 1fae3bc393a735375f041b606e2c8a5b7fcaa072 
>   
> metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java
>  5d683fb615e28c79d01e6c15571332076bcbbef0 
>   metastore/src/gen/thrift/gen-php/metastore/Types.php 
> b9af4efc5f8b7cdf19236db7d68865bdec8382a5 
>   metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 
> 21c039006fc05bc603fda0eeedc92174583f8403 
>   metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 
> c73593298bbddb46e0926b01ccb9c6fb5d880452 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> f7b2ed740f9c90771b6733a523f47cdbeb8b0896 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> 4774899119194d15795523bd547219deb79766f7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> 5ea000aba3a983fe339bc134c2e3d84a76563f5e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> d4024d2cd884c12e94efe469ea8f5d137850bf42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseUtils.java 
> 4546d43b365bcefd6adaf57570972ca038cf88cd 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MTable.java 
> 2a78ce9c0cd5fa4501564ca4760cfca61630eeb7 
>   metastore/src/model/package.jdo bfd6dddcec3359faf3a02905638d4e02b6e11bb6 
>   
> metastore/src/protobuf/org/apache/hadoop/hive/metastore/hbase/hbase_metastore_proto.proto
>  3f9e4c5fee99cc51c2de02d9865aca4fede657dd 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStorePartitionSpecs.java
>  61fe7e1d917b51abf1c1ad800d0d8f517c95c0ad 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 
> 04971591a68036cdc46c8755a063c32797dc4f8e 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsCache.java
>  6cd3a4643b63660861fde2edf6f94e976b1142d5 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsCacheWithBitVector.java
>  e0c4094fe3c4e963efbb3504d2a0e1a7d6e8ac98 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsExtrapolation.java
>  f4e55ed612eb6344c3408a5beba48b7ae6e18df4 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsNDVUniformDist.java
>  62918be10ce6fc693d994e25e7ad72ccefc647d5 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStore.java 
> fb0a8e701354615aee9fb862a25f63c7a81ee7e7 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreBitVector.java
>  b1dc542dffcaf4dd7d781c6acb5ed810f211bc40 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreCached.java
>  cfe9cd04468c83f8edd2bff9fcb4dc667ff79298 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java 
> 78715d8fa489c2ae4b88302d97e70cfd9ea0117c 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 493e1b389c14c792a32887c123b324013d698043 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> e477f24c05e95bce528963749e7ab339d0de92ec 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 
> ea908899a9f9145ff44e4dcb1a101696c596e442 
>   
> ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java
>  c850e4347eed2312cbc31a467473eda9fdff0ff4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRexExecutorImpl.java
>  f7958c66b134925d4c5dc5676269c26bec602f67 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/RelOptHiveTable.java 
> 4ebbb1342cb9632cae152dc6f382238fffc8ccab 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveFilter.java
>  0410c91fa3e1755a4970e6c9d8552fa046d4f3db 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveProject.java
>  3e0a9a625375b5fdebdac1d5ecb6e20ba79c1ca2 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveSemiJoin.java
>  d8996671ebba72be3e46e6ab96413748d299732b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveTableScan.java
>  cccbd2f8d5281748f1a12ab244a1c1f2c321305c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/HiveMaterializedViewFilterScanRule.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/MaterializedViewSubstitutionVisitor.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/views/SubstitutionVisitor.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
> f1f3bf93d29fce79485cbc0082464b9fa6df0f79 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> 55915a63be916b79dae022d76a4252ab1a18c64b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> 79e55b2de07983c7b799ff382b9c71ef14d25b43 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 
> d3b955c9eab96f58d0199b3903b77460a11af8e7 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java 
> 6830bda66ced5e3c8522aae8c0ed33c9a3e05c0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 
> 71aea3a12eff7e644eb6bef25d18eca754567e9a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite.q 
> PRE-CREATION 
>   
> ql/src/test/results/clientnegative/alter_view_as_select_with_partition.q.out 
> 4e43819a4b3dc89c4368b63bec2034a359d58d6b 
>   ql/src/test/results/clientpositive/alter_view_as_select.q.out 
> dc1814e45ceae940c7d71ae38bbfb76f01a410db 
>   ql/src/test/results/clientpositive/create_or_replace_view.q.out 
> f6f26d26cb97aec2e98b0d384026ea85d68f00a9 
>   ql/src/test/results/clientpositive/create_view.q.out 
> 12457b4a783d95f88d903b5083d4a684067c60ee 
>   ql/src/test/results/clientpositive/create_view_defaultformats.q.out 
> dbc4a2086e879dc7bea1a54cfdab46c7d48e9e2d 
>   ql/src/test/results/clientpositive/create_view_partitioned.q.out 
> 4373303c58d9429273f57de09c232e812f590f79 
>   ql/src/test/results/clientpositive/create_view_translate.q.out 
> 43b90621df2582e534e8746910df353cf7a50623 
>   ql/src/test/results/clientpositive/cteViews.q.out 
> eb3cfc03773351afcb3e2ac117ddc65eab1180a6 
>   ql/src/test/results/clientpositive/escape_comments.q.out 
> 0b8c5c521897ad5434adde36f3fe08b0a1c6149f 
>   ql/src/test/results/clientpositive/explain_ddl.q.out 
> e8438a132ce0b998baccef9de940e3cdf9ca6356 
>   ql/src/test/results/clientpositive/llap/cbo_rp_unionDistinct_2.q.out 
> 304d74f40d314a06d20180255d87796bcad9f4a3 
>   ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out 
> 985086d86344d25275068f8ae94a85197b2aa929 
>   ql/src/test/results/clientpositive/llap/subquery_views.q.out 
> 35e80aea93d38689f99c3ce4607a1adf43e6bcdb 
>   ql/src/test/results/clientpositive/llap/unionDistinct_2.q.out 
> 304d74f40d314a06d20180255d87796bcad9f4a3 
>   ql/src/test/results/clientpositive/llap/union_top_level.q.out 
> b4e4d9345a539d3b6689b177767adb7af8aea2dd 
>   ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/materialized_view_describe.q.out 
> 65d94d3c0e92a211fafac14897220e8acae8de64 
>   ql/src/test/results/clientpositive/spark/union_top_level.q.out 
> e1c7fc740a4319bcb5cbd4d05fa288bf35c515cc 
>   ql/src/test/results/clientpositive/subquery_views.q.out 
> 610bf245cfc25a54843ffe72b44c7ea01ca4d9e8 
>   ql/src/test/results/clientpositive/tez/unionDistinct_2.q.out 
> 304d74f40d314a06d20180255d87796bcad9f4a3 
>   ql/src/test/results/clientpositive/unicode_comments.q.out 
> 4872cd3809a34b91c18e891cb87fb5f3becaa4ab 
>   ql/src/test/results/clientpositive/view_alias.q.out 
> 78ff5e2d7c07c16a63be05095d3eda2ed35e9ca8 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 
> 70cb1266b1164c7acf63351a673e61844c31cd6d 
> 
> Diff: https://reviews.apache.org/r/53111/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>

Reply via email to