You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Zoltan Haindrich <ki...@rxd.hu> on 2020/05/04 08:52:09 UTC
Re: Review Request 72451: Add tests to cover database managed location
related DDL and fix minor issues
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72451/#review220565
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java
Line 106 (original), 106 (patched)
<https://reviews.apache.org/r/72451/#comment309057>
what's this `<if` stuff?
and there is a TABLE_NAME constant for "tableName"?
this templating stuff doesnt look good...instead of these strange ad-hoc templates (with if?) to generate statements for our own HiveQL language ....an SQL builder would be better and probably much easier to maintain...
- Zoltan Haindrich
On April 29, 2020, 6:03 p.m., Miklos Gergely wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72451/
> -----------------------------------------------------------
>
> (Updated April 29, 2020, 6:03 p.m.)
>
>
> Review request for hive and Zoltan Haindrich.
>
>
> Bugs: HIVE-23316
> https://issues.apache.org/jira/browse/HIVE-23316
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Database managed location was recently introduced, but no tests were added to cover it. also the following issues were fixed:
>
> - ALTER DATABASE ... SET MANAGEDLOCATION ... commands were not handled in a separate path as it should, as in DDL each command type have their own Analyzer, Desc, and Operation class
> - in case of setting the LOCATION or the MANAGEDLOCATION the location was not getting qualified as in the CREATE DATABASE command
> - in case of setting the LOCATION or the MANAGEDLOCATION it was not checked if this modification makes the two the same
> - some minor checkstyle issues were fixed as well
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetLocationDesc.java 16d28f2aa5
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetLocationOperation.java 0c4ade3538
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetManagedLocationAnalyzer.java a0e92eb3ef
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetManagedLocationDesc.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/alter/location/AlterDatabaseSetManagedLocationOperation.java PRE-CREATION
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseAnalyzer.java f87dd2a64e
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseDesc.java f3959f0b2a
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseOperation.java 444db0a8b8
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/desc/DescDatabaseDesc.java be0e5a963c
> ql/src/java/org/apache/hadoop/hive/ql/ddl/database/drop/DropDatabaseOperation.java a116a0e414
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableAnalyzer.java b362837439
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/show/ShowCreateTableOperation.java bf913442f9
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/drop/DropTableOperation.java 72b694f668
> ql/src/java/org/apache/hadoop/hive/ql/ddl/table/partition/drop/AlterTableDropPartitionOperation.java ae2c341c19
> ql/src/java/org/apache/hadoop/hive/ql/ddl/view/materialized/alter/rebuild/AlterMaterializedViewRebuildAnalyzer.java 4fb53785c2
> ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/LoadDatabase.java 52777f3b20
> ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/CreateDatabaseHandler.java 42fa88c5fb
> ql/src/test/queries/clientnegative/database_location_conflict.q PRE-CREATION
> ql/src/test/queries/clientnegative/database_location_conflict2.q PRE-CREATION
> ql/src/test/queries/clientnegative/database_location_conflict3.q PRE-CREATION
> ql/src/test/queries/clientpositive/database_location.q 8571958c29
> ql/src/test/results/clientnegative/database_location_conflict.q.out PRE-CREATION
> ql/src/test/results/clientnegative/database_location_conflict2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/database_location_conflict3.q.out PRE-CREATION
> ql/src/test/results/clientpositive/database_location.q.out f0567076a7
>
>
> Diff: https://reviews.apache.org/r/72451/diff/1/
>
>
> Testing
> -------
>
> Tests were added to cover the new DDL elements.
>
>
> Thanks,
>
> Miklos Gergely
>
>