[
https://issues.apache.org/jira/browse/HIVE-2093?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13022670#comment-13022670
]
jirapos...@reviews.apache.org commented on HIVE-2093:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/566/#review517
-----------------------------------------------------------
trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/566/#comment1053>
Spelling
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
<https://reviews.apache.org/r/566/#comment1058>
Why not make this a boolean function? I understand that the convention in
this class is to have functions that return 0, or something more than 0, but it
looks like all of the other functions can be boolean valued as well. Returning
an integer in this situation is confusing.
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
<https://reviews.apache.org/r/566/#comment1059>
Same deal here. Can you change this to return a boolean value?
trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
<https://reviews.apache.org/r/566/#comment1061>
You can eliminate a couple lines of code by using foreach syntax instead of
an explicit iterator.
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java
<https://reviews.apache.org/r/566/#comment1062>
I draw the line at nested ternary statements. Can you please use if/else
instead?
trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java
<https://reviews.apache.org/r/566/#comment1063>
Nice to have: move DATABASE to the beginning of the list.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
<https://reviews.apache.org/r/566/#comment1064>
Why do you need to initialize the lock manager here, but not above in
analyzeLockDatabase()?
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
<https://reviews.apache.org/r/566/#comment1065>
should be "unlock database statement"
trunk/ql/src/test/queries/clientnegative/lockneg6.q
<https://reviews.apache.org/r/566/#comment1066>
Please add some comments explaining what it is you're trying to test so
that I can quickly tell how this differs from lockneg5, lockneg7, etc, etc.
Please also consider changing the name of the file to something more
descriptive so that I don't even have to read the comments.
trunk/ql/src/test/results/clientnegative/authorization_fail_create_db.q.out
<https://reviews.apache.org/r/566/#comment1067>
Please make this less ambiguous and easier to parse by capitalizing SHOW
GRANT.
trunk/ql/src/test/results/clientnegative/exim_15_part_nonpart.q.out
<https://reviews.apache.org/r/566/#comment1068>
Any chance I can get a look at closed-source-hive? ;)
- Carl
On 2011-04-19 19:25:22, Siying Dong wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/566/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-04-19 19:25:22)
bq.
bq.
bq. Review request for hive, Yongqiang He and namit jain.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Still need to change some old tests' outputs.
bq.
bq.
bq. This addresses bug HIVE-2093.
bq. https://issues.apache.org/jira/browse/HIVE-2093
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java
1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java
1095164
bq.
trunk/ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java
1095164
bq.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 1095164
bq.
trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java
1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java
1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/LockDatabaseDesc.java
PRE-CREATION
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/ShowLocksDesc.java
1095164
bq. trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/UnlockDatabaseDesc.java
PRE-CREATION
bq. trunk/ql/src/test/queries/clientnegative/authorization_fail_create_db.q
PRE-CREATION
bq. trunk/ql/src/test/queries/clientnegative/authorization_fail_drop_db.q
PRE-CREATION
bq. trunk/ql/src/test/queries/clientnegative/lockneg6.q PRE-CREATION
bq. trunk/ql/src/test/queries/clientnegative/lockneg7.q PRE-CREATION
bq. trunk/ql/src/test/queries/clientnegative/lockneg8.q PRE-CREATION
bq. trunk/ql/src/test/queries/clientnegative/lockneg9.q PRE-CREATION
bq. trunk/ql/src/test/queries/clientpositive/database.q 1095164
bq.
trunk/ql/src/test/results/clientnegative/authorization_fail_create_db.q.out
PRE-CREATION
bq.
trunk/ql/src/test/results/clientnegative/authorization_fail_drop_db.q.out
PRE-CREATION
bq.
trunk/ql/src/test/results/clientnegative/database_create_already_exists.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/database_create_invalid_name.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/database_drop_does_not_exist.q.out
1095164
bq. trunk/ql/src/test/results/clientnegative/database_drop_not_empty.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_01_nonpart_over_loaded.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_02_all_part_over_overlap.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_03_nonpart_noncompat_colschema.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_04_nonpart_noncompat_colnumber.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_05_nonpart_noncompat_coltype.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_06_nonpart_noncompat_storage.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_07_nonpart_noncompat_ifof.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_08_nonpart_noncompat_serde.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_09_nonpart_noncompat_serdeparam.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_10_nonpart_noncompat_bucketing.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_11_nonpart_noncompat_sorting.q.out
1095164
bq. trunk/ql/src/test/results/clientnegative/exim_13_nonnative_import.q.out
1095164
bq. trunk/ql/src/test/results/clientnegative/exim_14_nonpart_part.q.out
1095164
bq. trunk/ql/src/test/results/clientnegative/exim_15_part_nonpart.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_16_part_noncompat_schema.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_17_part_spec_underspec.q.out
1095164
bq. trunk/ql/src/test/results/clientnegative/exim_18_part_spec_missing.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_19_external_over_existing.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_20_managed_location_over_existing.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_21_part_managed_external.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_23_import_exist_authfail.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_24_import_part_authfail.q.out
1095164
bq.
trunk/ql/src/test/results/clientnegative/exim_25_import_nonexist_authfail.q.out
1095164
bq. trunk/ql/src/test/results/clientnegative/lockneg6.q.out PRE-CREATION
bq. trunk/ql/src/test/results/clientnegative/lockneg7.q.out PRE-CREATION
bq. trunk/ql/src/test/results/clientnegative/lockneg8.q.out PRE-CREATION
bq. trunk/ql/src/test/results/clientnegative/lockneg9.q.out PRE-CREATION
bq. trunk/ql/src/test/results/clientpositive/add_part_exist.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/alter1.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/alter2.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/alter3.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/alter4.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/authorization_5.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/database.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/database_properties.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_00_nonpart_empty.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_01_nonpart.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/exim_02_00_part_empty.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_02_part.q.out 1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_03_nonpart_over_compat.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_04_all_part.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/exim_04_evolved_parts.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_05_some_part.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/exim_06_one_part.q.out 1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_07_all_part_over_nonoverlap.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_08_nonpart_rename.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_09_part_spec_nonoverlap.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_10_external_managed.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_11_managed_external.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_12_external_location.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_13_managed_location.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_14_managed_location_over_existing.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_15_external_part.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_16_part_external.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_17_part_managed.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/exim_18_part_external.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_19_part_external_location.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_20_part_managed_location.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_22_import_exist_authsuccess.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_23_import_part_authsuccess.q.out
1095164
bq.
trunk/ql/src/test/results/clientpositive/exim_24_import_nonexist_authsuccess.q.out
1095164
bq. trunk/ql/src/test/results/clientpositive/lock1.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/lock2.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/lock3.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/rename_column.q.out 1095164
bq. trunk/ql/src/test/results/clientpositive/show_tables.q.out 1095164
bq.
bq. Diff: https://reviews.apache.org/r/566/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Siying
bq.
bq.
> create/drop database should populate inputs/outputs and check concurrency and
> user permission
> ---------------------------------------------------------------------------------------------
>
> Key: HIVE-2093
> URL: https://issues.apache.org/jira/browse/HIVE-2093
> Project: Hive
> Issue Type: Bug
> Components: Metastore, Security
> Reporter: Namit Jain
> Assignee: Siying Dong
> Attachments: HIVE.2093.1.patch, HIVE.2093.2.patch, HIVE.2093.3.patch,
> HIVE.2093.4.patch
>
>
> concurrency and authorization are needed for create/drop table. Also to make
> concurrency work, it's better to have LOCK/UNLOCK DATABASE and SHOW LOCKS
> DATABASE
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
