> On April 15, 2016, 9:52 a.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?)

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.


- Nate


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


On April 15, 2016, 9:41 a.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, 9:41 a.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
> 
>

Reply via email to