Re: about rename table across database

[Na Li]

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 
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  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- <00>5732
>>> cloudera.com 
>>>
>>> [image: Cloudera] 
>>>
>>> [image: Cloudera on Twitter]  [image:
>>> Cloudera on Facebook]  [image:
>>> Cloudera
>>> on LinkedIn] 
>>> --
>>>
>>> On Tue, Apr 10, 2018 at 6:49 PM, Na Li  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 
>>> 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
>>> > >>
>>> > >>
>>> > >>
>>> > >>
>>> > >>
>>> > >>
>>> > >>
>>> > >>
>>> > >
>>> >
>>>
>>
>>
>

Re: about rename table across database

[Alexander Kolbasov]

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  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- <00>5732
>> cloudera.com 
>>
>> [image: Cloudera] 
>>
>> [image: Cloudera on Twitter]  [image:
>> Cloudera on Facebook]  [image:
>> Cloudera
>> on LinkedIn] 
>> --
>>
>> On Tue, Apr 10, 2018 at 6:49 PM, Na Li  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 > >
>> > 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  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
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >
>> >
>>
>
>

Re: about rename table across database

[Alexander Kolbasov]

Lina,

I have not checked your changes yet, I can take a look tomorrow.

On Tue, Apr 10, 2018 at 5:04 PM, Na Li  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- <00>5732
>> cloudera.com 
>>
>> [image: Cloudera] 
>>
>> [image: Cloudera on Twitter]  [image:
>> Cloudera on Facebook]  [image:
>> Cloudera
>> on LinkedIn] 
>> --
>>
>> On Tue, Apr 10, 2018 at 6:49 PM, Na Li  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 > >
>> > 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  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
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >>
>> > >
>> >
>>
>
>

Re: about rename table across database

[Na Li]

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- <00>5732
> cloudera.com 
>
> [image: Cloudera] 
>
> [image: Cloudera on Twitter]  [image:
> Cloudera on Facebook]  [image: Cloudera
> on LinkedIn] 
> --
>
> On Tue, Apr 10, 2018 at 6:49 PM, Na Li  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 
> > 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  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
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >
> >
>

Re: about rename table across database

[Kalyan Kumar Kalvagadda]

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- <00>5732
cloudera.com 

[image: Cloudera] 

[image: Cloudera on Twitter]  [image:
Cloudera on Facebook]  [image: Cloudera
on LinkedIn] 
--

On Tue, Apr 10, 2018 at 6:49 PM, Na Li  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 
> 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  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
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >
>

Re: about rename table across database

[Na Li]

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

Re: about rename table across database

[Alexander Kolbasov]

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