> 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.
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.
> On Dec. 12, 2017, 11:44 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
> > Lines 291 (patched)
> > <https://reviews.apache.org/r/64544/diff/1/?file=1914303#file1914303line292>
> >
> > You should create the FK declarations inside the table declarations.
I did this for a reason, but now I can't remember. I will test it and fix or
post my reason.
- Robert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64544/#review193564
-----------------------------------------------------------
On Dec. 12, 2017, 9:35 a.m., Robert Levas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64544/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2017, 9:35 a.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/orm/db/DDLTests.java
> 96cf64e53c
>
> 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/1/
>
>
> 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
>
>
