> 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. > > Jonathan Hurley wrote: > 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...
This could be dangerous, depending on where it fails. If it fails after removing the data from the Users table (`"delete from " + USERS_TABLE...`), then we will loose data when we drop the new table. - Robert ----------------------------------------------------------- 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 > >
