> On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > Mostly looks good. Some comments.
@Ashutosh, thanks for the feedback. Addressed the comments below. will update the diff. > On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > metastore/scripts/upgrade/derby/hive-schema-0.12.0.derby.sql, line 101 > > <https://reviews.apache.org/r/14169/diff/1/?file=352637#file352637line101> > > > > Name 'comment' has caused problems previously. I will suggest to name > > it VERSION_COMMENT, VCOMMENT or any other variation of it. ok. Changed it to VERSION_COMMENT. > On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java, > > line 47 > > <https://reviews.apache.org/r/14169/diff/1/?file=352655#file352655line47> > > > > Looks like this line can be removed. yes, fixed. > On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java, > > line 48 > > <https://reviews.apache.org/r/14169/diff/1/?file=352655#file352655line48> > > > > typo fixed > On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java, > > line 55 > > <https://reviews.apache.org/r/14169/diff/1/?file=352655#file352655line55> > > > > Can you name this variable version. I got confused thinking curVersion > > implies current version of jars (which was incorrect) Done > On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java, > > line 57 > > <https://reviews.apache.org/r/14169/diff/1/?file=352655#file352655line57> > > > > Will be good to do currVersion.trim() here. yes, given that we are reading it from a file, its safer to do that. Done. > On Sept. 17, 2013, 8:38 p.m., Ashutosh Chauhan wrote: > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java, line > > 5648 > > <https://reviews.apache.org/r/14169/diff/1/?file=352656#file352656line5648> > > > > Should this be if(strictValidation && ... ) That would be needed in the subsequent release. For example in 0.13, the schema version will be 0.12. When you upgrade from 12 to 13, the version comparison will fail. If the strict mode is not set, then we shouldn't throw exception. Thanks for catching that. Fixed. - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14169/#review26182 ----------------------------------------------------------- On Sept. 17, 2013, 6:13 a.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14169/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2013, 6:13 a.m.) > > > Review request for hive, Ashutosh Chauhan and Brock Noland. > > > Bugs: HIVE-3764 > https://issues.apache.org/jira/browse/HIVE-3764 > > > Repository: hive-git > > > Description > ------- > > This is a 0.12 specific patch. The trunk patch will include additional > metastore scripts which I will attach separately to the ticket. > > - Added a new table in the metastore schema to store the Hive version in the > metastore. > - Metastore handler compare the version stored in the schema with its own > version. If there's a mismatch, then it can either record the correct version > or raise error. The behavior is configurable via a new Hive config. This > config when set, also restrict dataNucleus to auto upgrade the schema. > - The new schema creation and upgrade scripts record the new version in the > metastore version table. > - Added 0.12 upgrade scripts for all supported DBs to creates the new table > version tables in 0.12 metastore schema > > The current patch has the verification turned off by default. I would prefer > to keep it enabled, though it require any add-hoc setup to explicitly disable > it (or create the metastore schema by running scripts). The default can be > changed or left as is as per the consensus. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 22149e4 > conf/hive-default.xml.template 9a3fc1d > metastore/scripts/upgrade/derby/014-HIVE-3764.derby.sql PRE-CREATION > metastore/scripts/upgrade/derby/hive-schema-0.12.0.derby.sql cce544f > metastore/scripts/upgrade/derby/upgrade-0.10.0-to-0.11.0.derby.sql cae7936 > metastore/scripts/upgrade/derby/upgrade-0.11.0-to-0.12.0.derby.sql 492cc93 > metastore/scripts/upgrade/derby/upgrade.order.derby PRE-CREATION > metastore/scripts/upgrade/mysql/014-HIVE-3764.mysql.sql PRE-CREATION > metastore/scripts/upgrade/mysql/hive-schema-0.12.0.mysql.sql 22a77fe > metastore/scripts/upgrade/mysql/upgrade-0.11.0-to-0.12.0.mysql.sql 375a05f > metastore/scripts/upgrade/mysql/upgrade.order.mysql PRE-CREATION > metastore/scripts/upgrade/oracle/014-HIVE-3764.oracle.sql PRE-CREATION > metastore/scripts/upgrade/oracle/hive-schema-0.12.0.oracle.sql 85a0178 > metastore/scripts/upgrade/oracle/upgrade-0.10.0-to-0.11.0.mysql.sql > PRE-CREATION > metastore/scripts/upgrade/oracle/upgrade-0.11.0-to-0.12.0.oracle.sql > a2d0901 > metastore/scripts/upgrade/oracle/upgrade.order.oracle PRE-CREATION > metastore/scripts/upgrade/postgres/014-HIVE-3764.postgres.sql PRE-CREATION > metastore/scripts/upgrade/postgres/hive-schema-0.12.0.postgres.sql 7b319ba > metastore/scripts/upgrade/postgres/upgrade-0.11.0-to-0.12.0.postgres.sql > 9da0a1b > metastore/scripts/upgrade/postgres/upgrade.order.postgres PRE-CREATION > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 39dda92 > > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreSchemaInfo.java > PRE-CREATION > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > a27243d > metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java e410c3a > > metastore/src/model/org/apache/hadoop/hive/metastore/model/MVersionTable.java > PRE-CREATION > metastore/src/model/package.jdo c42b5b0 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 8066784 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 0f9b16c > > metastore/src/test/org/apache/hadoop/hive/metastore/TestMetastoreVersion.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/14169/diff/ > > > Testing > ------- > > Added new tests for schema verification. Manually tested the upgrades using > derby and MySQL. > > > Thanks, > > Prasad Mujumdar > >
