[impala] 04/08: IMPALA-8434: retain tables and functions in altering database

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit c501b34fb2ea7378ae0b1b615d9610e93a2e5af0
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Tue Jul 23 17:09:09 2019 -0700

    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>
---
 .../java/org/apache/impala/catalog/ImpaladCatalog.java   | 16 +++++++++++++++-
 tests/metadata/test_ddl.py                               | 14 ++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
index 13cb620..05fb2b3 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
@@ -19,6 +19,7 @@ package org.apache.impala.catalog;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayDeque;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
@@ -388,16 +389,29 @@ public class ImpaladCatalog extends Catalog implements FeCatalog {
         existingDb.getCatalogVersion() < catalogVersion) {
       Db newDb = Db.fromTDatabase(thriftDb);
       newDb.setCatalogVersion(catalogVersion);
-      addDb(newDb);
       if (existingDb != null) {
         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. addTable and addFunction will add their
+        // versions back.
+        for (Table tbl: existingDb.getTables()) {
+          newDb.addTable(tbl);
+        }
+        for (List<Function> functionList: existingDb.getAllFunctions().values()) {
+          for (Function func: functionList) {
+            newDb.addFunction(func);
+          }
+        }
       } else {
         CatalogObjectVersionSet.INSTANCE.addVersion(catalogVersion);
       }
+      addDb(newDb);
     }
   }
 
diff --git a/tests/metadata/test_ddl.py b/tests/metadata/test_ddl.py
index e5dfc25..41502cc 100644
--- a/tests/metadata/test_ddl.py
+++ b/tests/metadata/test_ddl.py
@@ -237,6 +237,20 @@ class TestDdlStatements(TestDdlBase):
     assert len(properties) == 1
     assert {'foo_role': 'ROLE'} == properties
 
+  def test_metadata_after_alter_database(self, vector, unique_database):
+    self.client.execute("create table {0}.tbl (i int)".format(unique_database))
+    self.client.execute("create function {0}.f() returns int "
+                        "location '{1}/libTestUdfs.so' symbol='NoArgs'"
+                        .format(unique_database, WAREHOUSE))
+    self.client.execute("alter database {0} set owner user foo_user".format(
+      unique_database))
+    table_names = self.client.execute("show tables in {0}".format(
+      unique_database)).get_data()
+    assert "tbl" == table_names
+    func_names = self.client.execute("show functions in {0}".format(
+      unique_database)).get_data()
+    assert "INT\tf()\tNATIVE\ttrue" == func_names
+
   def test_alter_table_set_owner(self, vector, unique_database):
     table_name = "{0}.test_owner_tbl".format(unique_database)
     self.client.execute("create table {0}(i int)".format(table_name))