You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/07/24 00:15:20 UTC
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13904
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
IMPALA-8434: retain tables and functions in altering database
In the legacy catalog implementation (ImpaladCatalog), when altering a
database, the tables and functions in it will disappear until we run
INVALIDATE METADATA to reset the cache. The cause is that we just
replace the old Db object with the new one deserialized from the
TDatabase. We should migrate the existing tables and functions to the
new Db object.
Tests:
- Add test_metadata_after_alter_database for the bug.
Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
---
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/metadata/test_ddl.py
2 files changed, 27 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/13904/1
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 4: Code-Review+2
Thanks for making the suggested changes. Looks good to me.
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 20:18:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
IMPALA-8434: retain tables and functions in altering database
In the legacy catalog implementation (ImpaladCatalog), when altering a
database, the tables and functions in it will disappear until we run
INVALIDATE METADATA to reset the cache. The cause is that we just
replace the old Db object with the new one deserialized from the
TDatabase. We should migrate the existing tables and functions to the
new Db object.
Tests:
- Add test_metadata_after_alter_database for the bug.
- Run Core tests
Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Reviewed-on: http://gerrit.cloudera.org:8080/13904
Reviewed-by: Vihang Karajgaonkar <vi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/metadata/test_ddl.py
2 files changed, 29 insertions(+), 1 deletion(-)
Approvals:
Vihang Karajgaonkar: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@394
PS2, Line 394: CatalogObjectVersionSet.INSTANCE.updateVersions(
: existingDb.getCatalogVersion(), catalogVersion);
: CatalogObjectVersionSet.INSTANCE.removeAll(existingDb.getTables());
: CatalogObjectVersionSet.INSTANCE.removeAll(
: existingDb.getFunctions(null, new PatternMatcher()));
: // IMPALA-8434: add back the existing tables/functions. Note that their version
: // counters in CatalogObjectVersionSet have been decreased by the above removeAll
: // statements, meaning their references from the old db are deleted since the old
: // db object has been replaced by newDb.
: for (Table tbl: existingDb.getTables()) {
: newDb.addTable(tbl);
: }
> Db.addTable and Db.addFunction will finally add back the versions to Catalo
Oops I only went one level deep and checked tableCache_.add(table); Didn't realize that it was updating the CatalogObjectVersionSet underneath. Sorry for the false alarm.
http://gerrit.cloudera.org:8080/#/c/13904/2/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
http://gerrit.cloudera.org:8080/#/c/13904/2/tests/metadata/test_ddl.py@241
PS2, Line 241: self.client.execute("create table {0}.tbl (i int)".format(unique_database))
nit: how about merging this with the above test?
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 17:44:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@394
PS2, Line 394: CatalogObjectVersionSet.INSTANCE.updateVersions(
: existingDb.getCatalogVersion(), catalogVersion);
: CatalogObjectVersionSet.INSTANCE.removeAll(existingDb.getTables());
: CatalogObjectVersionSet.INSTANCE.removeAll(
: existingDb.getFunctions(null, new PatternMatcher()));
: // IMPALA-8434: add back the existing tables/functions. Note that their version
: // counters in CatalogObjectVersionSet have been decreased by the above removeAll
: // statements, meaning their references from the old db are deleted since the old
: // db object has been replaced by newDb.
: for (Table tbl: existingDb.getTables()) {
: newDb.addTable(tbl);
: }
This looks buggy to me. What does it mean to have tables in the db, without having their version number maintained in the CatalogObjectVersionSet ? Aren't we breaking the invariant that CatalogObjectVersionSet can get the correct min version across all the objects (invalidate metadata relies on it)? or am I misunderstanding something?
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 05:09:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3967/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jul 2019 00:55:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3975/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 Jul 2019 16:14:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4000/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 19:32:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13904
to look at the new patch set (#2).
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
IMPALA-8434: retain tables and functions in altering database
In the legacy catalog implementation (ImpaladCatalog), when altering a
database, the tables and functions in it will disappear until we run
INVALIDATE METADATA to reset the cache. The cause is that we just
replace the old Db object with the new one deserialized from the
TDatabase. We should migrate the existing tables and functions to the
new Db object.
Tests:
- Add test_metadata_after_alter_database for the bug.
- Run Core tests
Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
---
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/metadata/test_ddl.py
2 files changed, 27 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/13904/2
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Vihang Karajgaonkar, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13904
to look at the new patch set (#3).
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
IMPALA-8434: retain tables and functions in altering database
In the legacy catalog implementation (ImpaladCatalog), when altering a
database, the tables and functions in it will disappear until we run
INVALIDATE METADATA to reset the cache. The cause is that we just
replace the old Db object with the new one deserialized from the
TDatabase. We should migrate the existing tables and functions to the
new Db object.
Tests:
- Add test_metadata_after_alter_database for the bug.
- Run Core tests
Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
---
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M tests/metadata/test_ddl.py
2 files changed, 29 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/13904/3
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Jul 2019 03:28:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@392
PS2, Line 392: addDb(newDb);
Thanks to Vihang's comment in person. Will move this below after adding existing tables and functions to avoid race of seeing the empty new database.
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@394
PS2, Line 394: CatalogObjectVersionSet.INSTANCE.updateVersions(
: existingDb.getCatalogVersion(), catalogVersion);
: CatalogObjectVersionSet.INSTANCE.removeAll(existingDb.getTables());
: CatalogObjectVersionSet.INSTANCE.removeAll(
: existingDb.getFunctions(null, new PatternMatcher()));
: // IMPALA-8434: add back the existing tables/functions. Note that their version
: // counters in CatalogObjectVersionSet have been decreased by the above removeAll
: // statements, meaning their references from the old db are deleted since the old
: // db object has been replaced by newDb.
: for (Table tbl: existingDb.getTables()) {
: newDb.addTable(tbl);
: }
> Oops I only went one level deep and checked tableCache_.add(table); Didn't
Never mind :)
http://gerrit.cloudera.org:8080/#/c/13904/2/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
http://gerrit.cloudera.org:8080/#/c/13904/2/tests/metadata/test_ddl.py@241
PS2, Line 241: self.client.execute("create table {0}.tbl (i int)".format(unique_database))
> nit: how about merging this with the above test?
I think they are testing different points. The above one is for the functionality of altering owner for the database. This one is for checking metadata according to the bug. It'd be better to let them act as unit tests. So if someone fails them in the future, it'll be easier to know exactly what happens.
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 18:50:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4658/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 20:53:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8434: retain tables and functions in altering database
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13904 )
Change subject: IMPALA-8434: retain tables and functions in altering database
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13904/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@394
PS2, Line 394: CatalogObjectVersionSet.INSTANCE.updateVersions(
: existingDb.getCatalogVersion(), catalogVersion);
: CatalogObjectVersionSet.INSTANCE.removeAll(existingDb.getTables());
: CatalogObjectVersionSet.INSTANCE.removeAll(
: existingDb.getFunctions(null, new PatternMatcher()));
: // IMPALA-8434: add back the existing tables/functions. Note that their version
: // counters in CatalogObjectVersionSet have been decreased by the above removeAll
: // statements, meaning their references from the old db are deleted since the old
: // db object has been replaced by newDb.
: for (Table tbl: existingDb.getTables()) {
: newDb.addTable(tbl);
: }
> This looks buggy to me. What does it mean to have tables in the db, without
Db.addTable and Db.addFunction will finally add back the versions to CatalogObjectVersionSet.
https://github.com/apache/impala/blob/b2136c39fcafacec308dc9dd13ad13133596d05d/fe/src/main/java/org/apache/impala/catalog/Db.java#L158
https://github.com/apache/impala/blob/b2136c39fcafacec308dc9dd13ad13133596d05d/fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java#L75-L76
To avoid double counting on the versions, we need to remove them once. Note that CatalogObjectVersionSet is a multi-set.
--
To view, visit http://gerrit.cloudera.org:8080/13904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3dc9857fd2733e20cf10fbe17bb1a4670d7d015
Gerrit-Change-Number: 13904
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 Jul 2019 15:22:24 +0000
Gerrit-HasComments: Yes