> On April 15, 2016, 1:52 p.m., Nate Cole wrote: > > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql, lines 181-184 > > <https://reviews.apache.org/r/46269/diff/1/?file=1346529#file1346529line181> > > > > Let's be consistent. Either: > > pk_sc_desired_state > > fk_scds_... > > unq_scdesired... > > > > or > > PK_sc_desired > > FK_scds... > > UNQ_scdesired... > > > > I seem recall issues of case between the sql files and UpgradeCatalog* > > classes. Jonathan? > > Jonathan Hurley wrote: > I'm all for consistency. You're right that we used to have problems with > the case of the contraints. For PK's, we added some utility methods which > don't require us to know the name anymore: > > https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java#L569-L581 > > We could do something similar for FKs as well. > > Balázs Bence Sári wrote: > I've got a couple of questions: > - The method in DBAccessor only works with 3 databases. Can we rely on > that? > - I just learned that the name of the constraints are important during > upgrade. So I guess I'd need to revert any constraint renamings I've done and > only name constraints that had been unnamed so far. (pls. confirm) > - Existing constraints are mostly in PK_the_rest_is_lowecase format, but > some are in all lowercase format (also unique is sometimes noted as UQ and > sometimes UNQ). I always used the PK/FK/UQ_the_rest_is_lowercase format > because that seemed to be the most frequent. Are you ok to follow that > convenions? (Also, what should happen with existing all-lowercase > constraints? Would renaming them break upgrade scripts?) > > Nate Cole wrote: > I don't mind the PK/FK/UNQ_lower_name syntax myself, it's easy to > visually see "type" from "name" if you will. These scripts are create-only, > so changing them shouldn't break existing deployments. The concern is if > you've added new constraints, then yes, those need to be reflected in > UpgradeCatalog, but otherwise it seems safe. > > Balázs Bence Sári wrote: > I haven't introduced any new constraints, only renamed existing ones. > I am still a bit worried about renaming existing constraints (even naming > unnamed ones). While these scripts are create only, there will still be > inconsistency between fresh 2.4 installs and old installs upgraded to 2.4. It > won't be an issue right away, but it can become one when we will upgrade to > the next version after 2.4, unless we are 100% sure that after 2.4 upgrade > catalogs will never drop any constraint by name (e.g. by using Jonathan's new > API extended to all kinds of constraints).
Discussed with Jonathan: this Jira will only address the issues in the headline (inline constraints and name PK's). All other issues will be addressed in separare Jira's. - Balázs Bence ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46269/#review129114 ----------------------------------------------------------- On April 15, 2016, 1:41 p.m., Balázs Bence Sári wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46269/ > ----------------------------------------------------------- > > (Updated April 15, 2016, 1:41 p.m.) > > > Review request for Ambari, Jonathan Robie, Nate Cole, and Nahappan > Somasundaram. > > > Bugs: AMBARI-15915 > https://issues.apache.org/jira/browse/AMBARI-15915 > > > Repository: ambari > > > Description > ------- > > 1. Inlined alter table statements (except one to overcome a circular FK > dependency) > 2. Reorganized create table statemens to avoid forward references > 3. Named all unnamed constraints, shortened names to fit Oracle's 30 > character limit > 4. Unified constraint names across database flavors > 5. Wrote a JUnit test that checks DDL's for consistency (all constraints are > named, the same tables and constraints are defined for all database flavors) > > > Diffs > ----- > > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql f90ac96 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 2b214c4 > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql fc93372 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 870a8e8 > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 71d4813 > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 6e600c7 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql a3ea10d > > ambari-server/src/test/java/org/apache/ambari/server/orm/db/DDLTestUtils.java > PRE-CREATION > ambari-server/src/test/java/org/apache/ambari/server/orm/db/DDLTests.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/46269/diff/ > > > Testing > ------- > > 1. Run all java unit tests in ambari-server. All passed, except > ViewDirectoryWatcherTest.testDirectoryExtractionOnFileAdd(). Failure occured > to a hardcoded timeout value in the test being to low for my environment (I > raised the timeout from 7 secs to 20 and the test passed) > 2. Run DDL's with their respective DB environment. In cases where the DDL > contained additional things other than table/index cration and insertion > (e.g. creating databases and users) I only run the table/index creation part. > > > Thanks, > > Balázs Bence Sári > >
