hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vihang Karajgaonkar <vih...@cloudera.com>
Subject Re: Review Request 58994: HIVE-16556 : Modify schematool scripts to initialize and create METASTORE_DB_PROPERTIES table
Date Thu, 04 May 2017 21:57:29 GMT


> On May 4, 2017, 8:44 p.m., Naveen Gangam wrote:
> > beeline/src/java/org/apache/hive/beeline/HiveSchemaTool.java
> > Lines 960 (patched)
> > <https://reviews.apache.org/r/58994/diff/1/?file=1708358#file1708358line961>
> >
> >     Could you please give me the context for this change? Looks like only the test
code is using it for now. Thanks

When I was testing the SchemaTool was not logging the errors in the output file generated
by the test. So I thought of setting the error stream explicitly. But seems like it doesn't
really need to be set. Removed this change.


> On May 4, 2017, 8:44 p.m., Naveen Gangam wrote:
> > metastore/scripts/upgrade/derby/041-HIVE-16556.derby.sql
> > Lines 1 (patched)
> > <https://reviews.apache.org/r/58994/diff/1/?file=1708360#file1708360line1>
> >
> >     The last column name "DESC" is a bit ambiguious. It is also a reserved keyword
in Oracle (and perhaps others too).
> >     Can you change it to something a bit more descriptive. perhaps "DESCRIPTION"
or "COMMENT" if it fits your needs. COMMENT is used by some other tables in HMS schema as
well. So would be consistent as well (if it works for you that is)

Changed the column name to DESCRIPTION since COMMENT is also a reserved word in Oracle.


> On May 4, 2017, 8:44 p.m., Naveen Gangam wrote:
> > metastore/scripts/upgrade/derby/041-HIVE-16556.derby.sql
> > Lines 2 (patched)
> > <https://reviews.apache.org/r/58994/diff/1/?file=1708360#file1708360line2>
> >
> >     The new schema table is entirely detached from the rest of the tables, aka no
FK references. I am sure this is by design and me lacking the entire context.

Currently, there is no need for any Foreign keys for this table by design


> On May 4, 2017, 8:44 p.m., Naveen Gangam wrote:
> > metastore/scripts/upgrade/derby/hive-schema-3.0.0.derby.sql
> > Lines 109 (patched)
> > <https://reviews.apache.org/r/58994/diff/1/?file=1708361#file1708361line109>
> >
> >     nit: Could you move this to be just below the last current "CREATE TABLE" statement
but before the first "CREATE INDEX"? It just implies a certain order, thats all.

The SQLs for KEY_CONSTRAINTS table had CREATE INDEX, CREATE TABLE together and I thought the
convention was to keep all the SQLs of same table together. But looks like the convention
is to keep all the CREATE TABLEs, CREATE INDEXs and ALTER TABLE together. Changed it as per
the conventions for both METASTORE_DB_PROPERTIES and KEY_CONSTRAINTS tables


> On May 4, 2017, 8:44 p.m., Naveen Gangam wrote:
> > metastore/scripts/upgrade/mssql/026-HIVE-16556.mssql.sql
> > Lines 5 (patched)
> > <https://reviews.apache.org/r/58994/diff/1/?file=1708363#file1708363line5>
> >
> >     nit: I am guessing you had to insert quotes around "DESC" because it being a
reserved word. This makes the definition inconsistent with other columns in this table that
do not have the surrounding "". Changing the column name as suggested above should solve this
as well.

Renamed the column to DESCRIPTION for all the databases


- Vihang


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


On May 4, 2017, 9:57 p.m., Vihang Karajgaonkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58994/
> -----------------------------------------------------------
> 
> (Updated May 4, 2017, 9:57 p.m.)
> 
> 
> Review request for hive, Naveen Gangam, Sergio Pena, and Sahil Takiar.
> 
> 
> Bugs: HIVE-16556
>     https://issues.apache.org/jira/browse/HIVE-16556
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-16556 : Modify schematool scripts to initialize and create METASTORE_DB_PROPERTIES
table
> 
> 
> Diffs
> -----
> 
>   itests/hive-unit/src/test/java/org/apache/hive/beeline/TestSchemaTool.java 604c2345fcf04c877a8d257b9eef4610a58aea51

>   metastore/scripts/upgrade/derby/041-HIVE-16556.derby.sql PRE-CREATION 
>   metastore/scripts/upgrade/derby/hive-schema-3.0.0.derby.sql 8877681e05f9d9ecb3bec5d8579772b386dc7f08

>   metastore/scripts/upgrade/derby/upgrade-2.3.0-to-3.0.0.derby.sql 3bba5235eb630e4ee5d14bf7f163dfb7166df0ed

>   metastore/scripts/upgrade/mssql/026-HIVE-16556.mssql.sql PRE-CREATION 
>   metastore/scripts/upgrade/mssql/hive-schema-3.0.0.mssql.sql 98682a83760f89aec441af3f6cb5ff03f5251496

>   metastore/scripts/upgrade/mssql/upgrade-2.3.0-to-3.0.0.mssql.sql 94d18a34d37dea44a684242cbe1bdc5c3784d2e6

>   metastore/scripts/upgrade/mysql/041-HIVE-16556.mysql.sql PRE-CREATION 
>   metastore/scripts/upgrade/mysql/hive-schema-3.0.0.mysql.sql 4db2f34159d6650fab4179d1478c6c91c72b5351

>   metastore/scripts/upgrade/mysql/upgrade-2.3.0-to-3.0.0.mysql.sql e5d82e10160c2eab8571b3cd8f3078fbebcf3c5c

>   metastore/scripts/upgrade/oracle/041-HIVE-16556.oracle.sql PRE-CREATION 
>   metastore/scripts/upgrade/oracle/hive-schema-3.0.0.oracle.sql 7907c59f35a4575a614fdfd0419486156130ab23

>   metastore/scripts/upgrade/oracle/upgrade-2.3.0-to-3.0.0.oracle.sql 31c4f5dfbd94fd52317ad0cd65517e09925767f7

>   metastore/scripts/upgrade/postgres/040-HIVE-16556.postgres.sql PRE-CREATION 
>   metastore/scripts/upgrade/postgres/hive-schema-3.0.0.postgres.sql 49976d070d8a7ac825bb3920fa01f7d328aada9d

>   metastore/scripts/upgrade/postgres/upgrade-2.3.0-to-3.0.0.postgres.sql 2dd9bb9bfb607529f3ab9be4dccab34f26375e4f

> 
> 
> Diff: https://reviews.apache.org/r/58994/diff/2/
> 
> 
> Testing
> -------
> 
> 1. Used the testutils/metastore/metastore-upgrade-test.sh script to test against postgres,
mysql and derby database.
> 2. Manually tested against Oracle and MSSQL databases
> 3. Added unit-test to cover derby as well.
> 
> 
> Thanks,
> 
> Vihang Karajgaonkar
> 
>


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