hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesús Camacho Rodríguez <jcamachorodrig...@hortonworks.com>
Subject Re: Review Request 53111: HIVE-14496
Date Thu, 15 Dec 2016 12:21:21 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message