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
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >
>> >
>>
>
>

Reply via email to