> On Dec. 12, 2017, 11:44 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java > > Lines 175-192 (patched) > > <https://reviews.apache.org/r/64544/diff/1/?file=1914302#file1914302line175> > > > > Interesting .. is this idempotent? Typically nested-selects with > > inserts are not if they could violate PKs. Maybe you need to breakt this > > out into 2 queries and ensure you're only inserting when necessary? > > > > Then again, maybe I don't quite see what this query is doing and it's > > ok... Thought I'd flag it for discussion. > > Robert Levas wrote: > I assumed that this would fail if run multiple times. This is why there > is a conditional right before it: > ``` > // Migrate data from users table (if not yet upgraded) > if (!usersTableUpgraded()) { > dbAccessor.executeUpdate( > "insert into " + USER_AUTHENTICATION_TABLE + > "(" + USER_AUTHENTICATION_USER_AUTHENTICATION_ID_COLUMN + > ", " + USER_AUTHENTICATION_USER_ID_COLUMN + ", " + > USER_AUTHENTICATION_AUTHENTICATION_TYPE_COLUMN + ", " + > USER_AUTHENTICATION_AUTHENTICATION_KEY_COLUMN + ", " + > USER_AUTHENTICATION_CREATE_TIME_COLUMN + ", " + > USER_AUTHENTICATION_UPDATE_TIME_COLUMN + ")" + > ... > ); > } > ``` > > Breaking this apart into several queries and a set of insert statements > might help; but I thought that it would be difficult since the updated > UserEntity class would be out of sync with the original table. What do others > do in this situation? Maybe if I need to go this route, I would create a new > `user` table rather than upgrade the `users` table? In the end, I would drop > the old `user` table, but the OldUserEntitiy class will need to live for many > versions of Ambari.
My only concern here was being able to run this again after it fails. Should we just drop the new table in this method if it exists? Then we can keep this block as-is... - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64544/#review193564 ----------------------------------------------------------- On Dec. 13, 2017, 5:01 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64544/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2017, 5:01 p.m.) > > > Review request for Ambari, Attila Magyar, Balázs Bence Sári, Eugene > Chekanskiy, Jonathan Hurley, Nate Cole, Robert Nettleton, and Sandor Molnar. > > > Bugs: AMBARI-22577 > https://issues.apache.org/jira/browse/AMBARI-22577 > > > Repository: ambari > > > Description > ------- > > Migrate data from the users table (pre-Ambari 3.0.0) to the updated users > table and user_authentication tables. > > > Diffs > ----- > > ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java > 549c0fd7e8 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UserAuthenticationDAO.java > 513e78200d > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserAuthenticationEntity.java > 27514f648c > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProvider.java > 20a06ccd54 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java > 2de60957bc > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql e58a04e4b2 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql cc589e47f2 > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 7cd083db4c > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql ff232ebe3e > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql > d7c09f3381 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql b4929b308d > > ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java > 29f9d917e8 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderForDNWithSpaceTest.java > 10076b0876 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLdapAuthenticationProviderTest.java > 43d4d6b0c0 > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java > 747f99b618 > > > Diff: https://reviews.apache.org/r/64544/diff/2/ > > > Testing > ------- > > manually tested > > # Local test results: > ``` > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 27:21 min > [INFO] Finished at: 2017-12-11T17:30:31-05:00 > [INFO] Final Memory: 100M/2019M > [INFO] > ------------------------------------------------------------------------ > ``` > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
