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