Sasha, I agree with you on the desired behavior. The current fix does cover what you mentioned, but may have a bug. I will add more testing cases and make sure it renames the table correctly, and does not rename other tables by accident.
Thanks, Lina On Wed, Apr 11, 2018 at 2:56 PM, Alexander Kolbasov <ak...@cloudera.com> wrote: > Lina, > > I looked at your changes. I agree with you that the original code didn't > handle this case correctly, but I think that the new code doesn't do it > either. > > What we want is to rename the key in the map - rename oldDb.tblName into > newDb.tblName which means that we need get the value of the old key, delete > the old key and insert it into the new key. We should also have an antry > for the new DB. The rename part is missing in your change - or I am missing > something. > > What do you think? > > - Sasha > > On Tue, Apr 10, 2018 at 5:04 PM, Na Li <lina...@cloudera.com> wrote: > >> Sasha, >> >> are you OK with my code change in https://reviews.apache.org/r/65768/? >> >> Thanks, >> >> Lina >> >> On Tue, Apr 10, 2018 at 6:54 PM, Kalyan Kumar Kalvagadda < >> kkal...@cloudera.com> wrote: >> >>> alter table could do couple of things >>> 1. alter the table name (alter table db1.tb1 rename to db1.tb2) >>> 2. move the table from one database to another one (alter table >>> db1.tb2 rename to db2.tb2;) >>> 3. both (alter table db1.tb2 rename to db2.tb3) >>> >>> From what I understand, difference in prevDbName and newDbName means >>> that table is moved from one database to another. >>> >>> >>> >>> *Thanks,Kalyan Kumar Kalvagadda* | Software Engineer >>> t. (469) 279- <0000000000>5732 >>> cloudera.com <https://www.cloudera.com> >>> >>> [image: Cloudera] <https://www.cloudera.com/> >>> >>> [image: Cloudera on Twitter] <https://twitter.com/cloudera> [image: >>> Cloudera on Facebook] <https://www.facebook.com/cloudera> [image: >>> Cloudera >>> on LinkedIn] <https://www.linkedin.com/company/cloudera> >>> ------------------------------ >>> >>> On Tue, Apr 10, 2018 at 6:49 PM, Na Li <lina...@cloudera.com> wrote: >>> >>> > Sasha, >>> > >>> > We can. And I added the test case in e2e tests, and it works. >>> > >>> > On Tue, Apr 10, 2018 at 5:34 PM, Alexander Kolbasov < >>> ak...@cloudera.com> >>> > wrote: >>> > >>> > > I think you are right - there is no reason to do that. I am not sure >>> > > whether we can actually have changed DB name in ALTER TABLE event at >>> all. >>> > > >>> > > On Tue, Apr 10, 2018 at 12:58 PM, Na Li <lina...@cloudera.com> >>> wrote: >>> > > >>> > >> Sasha, >>> > >> >>> > >> >>> > >> In FullUpdateModifier.alterTable(), why did you have the section >>> that >>> > >> "Walk through all tables and rename DB part of the AUTH name"? >>> > >> >>> > >> This is alter table processing, so we should only process one >>> table, not >>> > >> all tables in a database. Also, in NotificationProcessor, we don't >>> > change >>> > >> all tables in a database if the database name changes. >>> > >> >>> > >> Can you review https://reviews.apache.org/r/65768/? I removed that >>> > >> section. >>> > >> >>> > >> Thanks, >>> > >> >>> > >> Lina >>> > >> >>> > >> >>> > >> >>> > >> >>> > >> >>> > >> >>> > >> >>> > >> >>> > > >>> > >>> >> >> >
