You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Balázs Bence Sári <bs...@hortonworks.com> on 2016/04/15 15:41:22 UTC

Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

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

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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Nate Cole <nc...@hortonworks.com>.

> On April 15, 2016, 10:02 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql, line 1572
> > <https://reviews.apache.org/r/46269/diff/1/?file=1346534#file1346534line1572>
> >
> >     Quartz tables are not ours and I don't know the implications of changing them. Let's stick to our own.
> 
> Balázs Bence Sári wrote:
>     Ok. I'll revert all changes to them (including inlining constraints if any). Also, is there anything else not owned by us (e.g.alert fwk, kerberos workflow) ?

I think it would be a good time to remove those pesky RCA tables.  They're an old artifact that no one's using.  (Test a full deployment just to be sure)


- Nate


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


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Balázs Bence Sári <bs...@hortonworks.com>.

> On April 15, 2016, 2:02 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql, line 1572
> > <https://reviews.apache.org/r/46269/diff/1/?file=1346534#file1346534line1572>
> >
> >     Quartz tables are not ours and I don't know the implications of changing them. Let's stick to our own.

Ok. I'll revert all changes to them (including inlining constraints if any). Also, is there anything else not owned by us (e.g.alert fwk, kerberos workflow) ?


- Balázs Bence


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


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46269/#review129118
-----------------------------------------------------------




ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql (line 1326)
<https://reviews.apache.org/r/46269/#comment192587>

    Quartz tables are not ours and I don't know the implications of changing them. Let's stick to our own.


- Jonathan Hurley


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Balázs Bence Sári <bs...@hortonworks.com>.

> 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).
> 
> Balázs Bence Sári wrote:
>     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.

I postponed renaming existing constraints until we have the API ready in DBAccessor to make upgrade catalogs constraint name agnostic.


- Balázs Bence


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


On April 20, 2016, 8:25 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 20, 2016, 8:25 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
> 
>


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Balázs Bence Sári <bs...@hortonworks.com>.

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

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


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Jonathan Hurley <jh...@hortonworks.com>.

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

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.


- Jonathan


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Balázs Bence Sári <bs...@hortonworks.com>.

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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Balázs Bence Sári <bs...@hortonworks.com>.

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

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


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Nate Cole <nc...@hortonworks.com>.

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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46269/#review129114
-----------------------------------------------------------




ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql (lines 181 - 184)
<https://reviews.apache.org/r/46269/#comment192585>

    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?


- Nate Cole


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


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46269/#review129820
-----------------------------------------------------------


Ship it!




Ship It!

- Nate Cole


On April 20, 2016, 4:25 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 20, 2016, 4:25 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
> 
>


Re: Review Request 46269: SQL constraints: Inline constraints and name them in CREATE table

Posted by Balázs Bence Sári <bs...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46269/
-----------------------------------------------------------

(Updated April 20, 2016, 8:25 p.m.)


Review request for Ambari, Jonathan Robie, Nate Cole, and Nahappan Somasundaram.


Changes
-------

1. Quartz tables were not touched in the 2nd patch (except in the Derby DDL which contained bad column names)
2. Only PK's are named, other unnamed constraints remained as is (according to discussion with Jonathan)
3. In cases where there were missing columns/constraints, the Postgres DDL was taken as master.
4. I Changed the constraint naming algorithm: names were only shortened if they hit Oracle's 30 character limit.
5. Tests were updated.
6. Test run result: all Java unit tests passed in ambari-server.


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 (updated)
-----

  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