----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50881/#review145051 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java (lines 564 - 588) <https://reviews.apache.org/r/50881/#comment211211> Oracle sucks. Because EclipseLink hard codes the order of the constraints, we can't leverage FieldDefinition because the order would be wrong. Instead, we have to: - Create the column as nullable - Update the field manually via an UPDATE statement - Enforce the correct NULL constraint - Enforce the DEFAULT constraint I know it's not atomic, however the statements aside from the column creation are idempotent by nature, so I think that's fine. ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java (lines 1233 - 1257) <https://reviews.apache.org/r/50881/#comment211212> We needed this for Oracle only in order to actually set the DEFAULT constraint after the column was created. However, I figured it was easy enough to just add the correct syntax for other platforms as well. ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java (lines 209 - 211) <https://reviews.apache.org/r/50881/#comment211213> Only append the DEFAULT constraint if the platform supports it coming after the NULL constraint. Once again, Oracle sucks. - Jonathan Hurley On Aug. 7, 2016, 4:10 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50881/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2016, 4:10 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Robert Levas, and Robert > Nettleton. > > > Bugs: AMBARI-18057 > https://issues.apache.org/jira/browse/AMBARI-18057 > > > Repository: ambari > > > Description > ------- > > Noticed the following on an upgraded cluster: > This is not related to EU it seems: > > ``` > java.lang.NullPointerException > at > org.apache.ambari.server.controller.internal.AlertResourceProvider.toResource(AlertResourceProvider.java:277) > at > org.apache.ambari.server.controller.internal.AlertResourceProvider.getResources(AlertResourceProvider.java:234) > at > org.apache.ambari.server.controller.internal.AlertResourceProvider.queryForResources(AlertResourceProvider.java:160) > ``` > > It looks like this is a problem with database consistency, namely that our > upgrade logic does NOT properly enforce non-null and default values! > ``` > //TODO workaround for default values, possibly we will have full support later > if (columnInfo.getDefaultValue() != null) { > columnInfo.setNullable(true); > } > ``` > > Turns out we're doing at least 2 things very wrong here when changing the DB > on Ambari upgrade: > - Not enforcing non-NULL constraint on columns > - Not enforcing a default constraint on column > > This is most likely due to limitations in EclipseLink's {{TableDefinition}} > and {{FieldDefinition}} classes which we rely on for our upgrade logic. > > > Diffs > ----- > > ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java > 5f126f6 > > ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java > ea5f496 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java > 77f5acc > > ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/DbmsHelper.java > 30c06fb > > ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java > 21fa361 > > ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java > 88ef8fe > > ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java > ac8bea1 > > Diff: https://reviews.apache.org/r/50881/diff/ > > > Testing > ------- > > Tested an upgrade from Ambari 2.2 / HDP 2.3 to Ambari 2.4.0 on: > - MySQL > - Postgres > - Oracle > > Still working on SQL Server, but figure I'd get the review up while I was > setting up yet another environment. > > > Thanks, > > Jonathan Hurley > >
