You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Navis Ryu <na...@nexr.com> on 2014/08/01 03:55:47 UTC

Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

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

(Updated Aug. 1, 2014, 1:55 a.m.)


Review request for hive and Thejas Nair.


Changes
-------

Rebased to trunk


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


Repository: hive-git


Description
-------

Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 


Diffs (updated)
-----

  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b74868b 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4f186f4 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java cba5cfa 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 40d910c 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 4cf4522 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java ae87aac 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 11a6d07 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 51838ae 
  ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
  ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
  ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
  ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
  ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
  ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
  ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
  ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
  ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
  ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
  ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
  ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
  ql/src/test/results/clientpositive/input3.q.out 547449c 
  ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
  ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
  ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
  ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
  ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 

Diff: https://reviews.apache.org/r/23674/diff/


Testing
-------


Thanks,

Navis Ryu


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 2068
> > <https://reviews.apache.org/r/23674/diff/2/?file=647688#file647688line2068>
> >
> >     can you please add javadoc for this and getTableName, what it does is not obvious from function name.
> >     Maybe also change the argument name to dbTableName ?
> >

Done


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java, line 313
> > <https://reviews.apache.org/r/23674/diff/2/?file=647692#file647692line313>
> >
> >     Since this is a common utility function, how about also throwing an error if child count is > 2 ? Maybe use Preconditions like HIVE-7561 does.
> >

I think it would not be happened because it's not defined in grammar possibly to be like that. But ok.


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java, line 94
> > <https://reviews.apache.org/r/23674/diff/2/?file=647707#file647707line94>
> >
> >     Isn't it better to represent the columns as a set instead of list, as multiple columns with same name in this object does not make sense ?
> >     Same for other places in this patch where columns has been changed from a set to list.

HivePrivilegeObject compares columns by iteration. If columns is not ordered somehow, it seemed not a valid comparison. I didn't have a idea to compare two column sets, I just replaced it to a sorted list, which felt easier that that. Any idea?


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java, line 607
> > <https://reviews.apache.org/r/23674/diff/2/?file=647687#file647687line607>
> >
> >     doesn't the default authorization mode support columns in show-grants ? It is there in ShowGrantDesc
> >

I've moved columns in ShowGrantDesc to PrivilegeObjectDesc, which seemed more neat, imho. Isn't it?


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/test/results/clientpositive/temp_table_precedence.q.out, line 179
> > <https://reviews.apache.org/r/23674/diff/2/?file=647728#file647728line179>
> >
> >     Some of the test q.out updates show that this is fixing issue for some cases being fixed in the jira. Are there any .q files that need to be updated ? For example, any .q files to be updated for describe/show partition/show tblproperties ?
> >

I've not fixed any codes related to drop table but the result looked correct and neglected that. Seemed need a research on this.


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java, line 2110
> > <https://reviews.apache.org/r/23674/diff/2/?file=647694#file647694line2110>
> >
> >     how about using BaseSemanticAnalyzer. getQualifiedTableName and having this check for 'duplicate declaration' there ?
> >     
> >

It's not TOK_TABNAME (which is TOK_FROM identifier? (identifier|StringLiteral)?) and seemed not replaced with getQualifiedTableName()


- Navis


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


On Aug. 1, 2014, 1:55 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23674/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 1:55 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-4064
>     https://issues.apache.org/jira/browse/HIVE-4064
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b74868b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4f186f4 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java cba5cfa 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 40d910c 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 4cf4522 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java ae87aac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 11a6d07 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 51838ae 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
>   ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
>   ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
>   ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
>   ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
>   ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
>   ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
>   ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
>   ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
>   ql/src/test/results/clientpositive/input3.q.out 547449c 
>   ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
>   ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
>   ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
>   ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
>   ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 
> 
> Diff: https://reviews.apache.org/r/23674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Thejas Nair <th...@hortonworks.com>.

> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java, line 607
> > <https://reviews.apache.org/r/23674/diff/2/?file=647687#file647687line607>
> >
> >     doesn't the default authorization mode support columns in show-grants ? It is there in ShowGrantDesc
> >
> 
> Navis Ryu wrote:
>     I've moved columns in ShowGrantDesc to PrivilegeObjectDesc, which seemed more neat, imho. Isn't it?

Yes, thats certainly better.


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java, line 2110
> > <https://reviews.apache.org/r/23674/diff/2/?file=647694#file647694line2110>
> >
> >     how about using BaseSemanticAnalyzer. getQualifiedTableName and having this check for 'duplicate declaration' there ?
> >     
> >
> 
> Navis Ryu wrote:
>     It's not TOK_TABNAME (which is TOK_FROM identifier? (identifier|StringLiteral)?) and seemed not replaced with getQualifiedTableName()

Thanks for clarifying!


> On Aug. 5, 2014, 5:09 a.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java, line 94
> > <https://reviews.apache.org/r/23674/diff/2/?file=647707#file647707line94>
> >
> >     Isn't it better to represent the columns as a set instead of list, as multiple columns with same name in this object does not make sense ?
> >     Same for other places in this patch where columns has been changed from a set to list.
> 
> Navis Ryu wrote:
>     HivePrivilegeObject compares columns by iteration. If columns is not ordered somehow, it seemed not a valid comparison. I didn't have a idea to compare two column sets, I just replaced it to a sorted list, which felt easier that that. Any idea?

Sounds fine. Maybe we can do a copy and sort as part of the constructor of this HivePrivilegeObject, instead of relying on the argument being sorted. That is likely to avoid potential bugs. But that can be done as part of separate jira.


- Thejas


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


On Aug. 1, 2014, 1:55 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23674/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 1:55 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-4064
>     https://issues.apache.org/jira/browse/HIVE-4064
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b74868b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4f186f4 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java cba5cfa 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 40d910c 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 4cf4522 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java ae87aac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 11a6d07 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 51838ae 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
>   ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
>   ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
>   ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
>   ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
>   ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
>   ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
>   ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
>   ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
>   ql/src/test/results/clientpositive/input3.q.out 547449c 
>   ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
>   ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
>   ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
>   ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
>   ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 
> 
> Diff: https://reviews.apache.org/r/23674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23674/#review49498
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
<https://reviews.apache.org/r/23674/#comment86575>

    doesn't the default authorization mode support columns in show-grants ? It is there in ShowGrantDesc
    



ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/23674/#comment86591>

    can you please add javadoc for this and getTableName, what it does is not obvious from function name.
    Maybe also change the argument name to dbTableName ?
    



ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
<https://reviews.apache.org/r/23674/#comment86590>

    Since this is a common utility function, how about also throwing an error if child count is > 2 ? Maybe use Preconditions like HIVE-7561 does.
    



ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
<https://reviews.apache.org/r/23674/#comment86592>

    how about using BaseSemanticAnalyzer. getQualifiedTableName and having this check for 'duplicate declaration' there ?
    
    



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
<https://reviews.apache.org/r/23674/#comment86707>

    Isn't it better to represent the columns as a set instead of list, as multiple columns with same name in this object does not make sense ?
    Same for other places in this patch where columns has been changed from a set to list.



ql/src/test/results/clientpositive/temp_table_precedence.q.out
<https://reviews.apache.org/r/23674/#comment86708>

    Some of the test q.out updates show that this is fixing issue for some cases being fixed in the jira. Are there any .q files that need to be updated ? For example, any .q files to be updated for describe/show partition/show tblproperties ?
    


- Thejas Nair


On Aug. 1, 2014, 1:55 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23674/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2014, 1:55 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-4064
>     https://issues.apache.org/jira/browse/HIVE-4064
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b74868b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 4f186f4 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java cba5cfa 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 40d910c 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 4cf4522 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a7e50ad 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java ae87aac 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 11a6d07 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 51838ae 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
>   ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
>   ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
>   ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
>   ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
>   ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
>   ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
>   ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
>   ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
>   ql/src/test/results/clientpositive/input3.q.out 547449c 
>   ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
>   ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
>   ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
>   ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
>   ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 
> 
> Diff: https://reviews.apache.org/r/23674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23674/#review50141
-----------------------------------------------------------

Ship it!


Ship It!

- Thejas Nair


On Aug. 11, 2014, 12:53 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23674/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 12:53 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-4064
>     https://issues.apache.org/jira/browse/HIVE-4064
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java ea866c5 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e689d0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 760777a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 74b1432 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java ea6ddbf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 376e040 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java d22b1f6 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 39b032e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java 2e32fee 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 989d0b5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 7b86414 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
>   ql/src/test/queries/clientpositive/alter_rename_table.q PRE-CREATION 
>   ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
>   ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
>   ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
>   ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
>   ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
>   ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
>   ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
>   ql/src/test/results/clientpositive/alter_rename_table.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
>   ql/src/test/results/clientpositive/input3.q.out 547449c 
>   ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
>   ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
>   ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
>   ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
>   ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 
> 
> Diff: https://reviews.apache.org/r/23674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Navis Ryu <na...@nexr.com>.

> On Aug. 11, 2014, 3:51 a.m., Lefty Leverenz wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 2071
> > <https://reviews.apache.org/r/23674/diff/3/?file=657207#file657207line2071>
> >
> >     "which in the form" should be "which is in the form"

fixed.


> On Aug. 11, 2014, 3:51 a.m., Lefty Leverenz wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 2086
> > <https://reviews.apache.org/r/23674/diff/3/?file=657207#file657207line2086>
> >
> >     "which in the form" should be "which is in the form"

fixed.


> On Aug. 11, 2014, 3:51 a.m., Lefty Leverenz wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java, lines 103-105
> > <https://reviews.apache.org/r/23674/diff/3/?file=657209#file657209line103>
> >
> >     should "does contain database name" be "does not contain database name"?

You are right. Fixed again. Thanks.


- Navis


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


On Aug. 11, 2014, 12:53 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23674/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 12:53 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-4064
>     https://issues.apache.org/jira/browse/HIVE-4064
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java ea866c5 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e689d0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 760777a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 74b1432 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java ea6ddbf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 376e040 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java d22b1f6 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 39b032e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java 2e32fee 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 989d0b5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 7b86414 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
>   ql/src/test/queries/clientpositive/alter_rename_table.q PRE-CREATION 
>   ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
>   ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
>   ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
>   ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
>   ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
>   ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
>   ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
>   ql/src/test/results/clientpositive/alter_rename_table.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
>   ql/src/test/results/clientpositive/input3.q.out 547449c 
>   ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
>   ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
>   ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
>   ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
>   ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 
> 
> Diff: https://reviews.apache.org/r/23674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Lefty Leverenz <le...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23674/#review50132
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/23674/#comment87707>

    "which in the form" should be "which is in the form"



ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/23674/#comment87708>

    "which in the form" should be "which is in the form"



ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java
<https://reviews.apache.org/r/23674/#comment87715>

    should "does contain database name" be "does not contain database name"?


- Lefty Leverenz


On Aug. 11, 2014, 12:53 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23674/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2014, 12:53 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-4064
>     https://issues.apache.org/jira/browse/HIVE-4064
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java ea866c5 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e689d0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 760777a 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 74b1432 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java ea6ddbf 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 376e040 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java d22b1f6 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 39b032e 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java 2e32fee 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 989d0b5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 7b86414 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
>   ql/src/test/queries/clientpositive/alter_rename_table.q PRE-CREATION 
>   ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
>   ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
>   ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
>   ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
>   ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
>   ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
>   ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
>   ql/src/test/results/clientpositive/alter_rename_table.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
>   ql/src/test/results/clientpositive/input3.q.out 547449c 
>   ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
>   ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
>   ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
>   ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
>   ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 
> 
> Diff: https://reviews.apache.org/r/23674/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Re: Review Request 23674: Handle db qualified names consistently across all HiveQL statements

Posted by Navis Ryu <na...@nexr.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23674/
-----------------------------------------------------------

(Updated Aug. 11, 2014, 12:53 a.m.)


Review request for hive and Thejas Nair.


Changes
-------

Supports renaming table cross database.


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


Repository: hive-git


Description
-------

Hive doesn't consistently handle db qualified names across all HiveQL statements. While some HiveQL statements such as SELECT support DB qualified names, other such as CREATE INDEX doesn't. 


Diffs (updated)
-----

  itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveAuthorizerCheckInvocation.java c91b15c 
  itests/util/src/main/java/org/apache/hadoop/hive/ql/hooks/CheckColumnAccessHook.java 14fc430 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java ea866c5 
  metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 6e689d0 
  metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 5a56ced 
  metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 760777a 
  metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 74b1432 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java ea6ddbf 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 376e040 
  ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java d22b1f6 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 39b032e 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java 2e32fee 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java 989d0b5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 22945e3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnAccessInfo.java 939dc65 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 67a3aa7 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g ab1188a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IndexUpdater.java 856ec2f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 7b86414 
  ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java 826bdf3 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterIndexDesc.java 0318e4b 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableAlterPartDesc.java cf67e16 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java 541675c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 9417220 
  ql/src/java/org/apache/hadoop/hive/ql/plan/RenamePartitionDesc.java 1b5fb9e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java fe6a91e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java aa88153 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java 5c94217 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java 9e9ef71 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java fbc0090 
  ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 98c2924 
  ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 5f32d5f 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java 93901ec 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java ab0d80e 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV1.java fd827ad 
  ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestPrivilegesV2.java 9499986 
  ql/src/test/queries/clientpositive/alter_rename_table.q PRE-CREATION 
  ql/src/test/results/clientnegative/alter_concatenate_indexed_table.q.out 500d45d 
  ql/src/test/results/clientnegative/alter_view_failure6.q.out cfbaca8 
  ql/src/test/results/clientnegative/merge_negative_1.q.out 95f6678 
  ql/src/test/results/clientnegative/merge_negative_2.q.out b3422e1 
  ql/src/test/results/clientnegative/show_columns3.q.out 09068b7 
  ql/src/test/results/clientnegative/show_tableproperties1.q.out ca54088 
  ql/src/test/results/clientnegative/temp_table_index.q.out 8ec5c0a 
  ql/src/test/results/clientpositive/alter_rename_table.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/drop_multi_partitions.q.out eae57f3 
  ql/src/test/results/clientpositive/input3.q.out 547449c 
  ql/src/test/results/clientpositive/insert2_overwrite_partitions.q.out 21bd257 
  ql/src/test/results/clientpositive/show_create_table_db_table.q.out 0119471 
  ql/src/test/results/clientpositive/show_tblproperties.q.out 80db5e4 
  ql/src/test/results/clientpositive/temp_table_names.q.out 940684c 
  ql/src/test/results/clientpositive/temp_table_precedence.q.out 1075b2c 

Diff: https://reviews.apache.org/r/23674/diff/


Testing
-------


Thanks,

Navis Ryu