You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2022/03/23 06:26:06 UTC

[impala] annotated tag 3.4.1-rc2 created (now 2de7132)

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

stigahuang pushed a change to annotated tag 3.4.1-rc2
in repository https://gitbox.apache.org/repos/asf/impala.git.


      at 2de7132  (tag)
 tagging 2c7e69f3b5f59d2ec65e3c5bf57ef1add1977278 (commit)
 replaces 3.4.1-rc1
      by stiga-huang
      on Wed Mar 23 14:25:46 2022 +0800

- Log -----------------------------------------------------------------
3.4.1 release candidate 2
-----------------------------------------------------------------------

This annotated tag includes the following new commits:

     new 134b649  Revert "IMPALA-9242: Filter privileges before returning them to Sentry"
     new 2c7e69f  Prepare 3.4.1 RC2

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[impala] 02/02: Prepare 3.4.1 RC2

Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to annotated tag 3.4.1-rc2
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 2c7e69f3b5f59d2ec65e3c5bf57ef1add1977278
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Wed Mar 23 14:22:39 2022 +0800

    Prepare 3.4.1 RC2
    
    Change-Id: Id6e3387ac6c4989f03b456fc12237b84387899f3
---
 bin/save-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bin/save-version.sh b/bin/save-version.sh
index c94df07..eedaf2d 100755
--- a/bin/save-version.sh
+++ b/bin/save-version.sh
@@ -22,7 +22,7 @@
 # "-INTERNAL" appended. Parts of the code will look for this to distinguish
 # between released and internal versions.
 VERSION=3.4.1-RELEASE
-GIT_HASH=0d55beb121ba2fdf2c2ec68d1f26c5a8de028803
+GIT_HASH=134b6492edece7d723bd48890dc934e331988a16
 if [ -z $GIT_HASH ]
 then
   GIT_HASH="Could not obtain git hash"

[impala] 01/02: Revert "IMPALA-9242: Filter privileges before returning them to Sentry"

Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to annotated tag 3.4.1-rc2
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 134b6492edece7d723bd48890dc934e331988a16
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Tue Mar 22 21:11:09 2022 +0800

    Revert "IMPALA-9242: Filter privileges before returning them to Sentry"
    
    This reverts commit e7d10df2ecaf14f244eb32224e2c8099f2f0d8cf.
---
 .../sentry/SentryAuthorizationPolicy.java          |  85 +------
 .../java/org/apache/impala/catalog/Principal.java  |  51 +----
 .../apache/impala/catalog/PrincipalPrivilege.java  |   4 +-
 .../impala/catalog/PrincipalPrivilegeTree.java     | 248 ---------------------
 .../impala/catalog/PrincipalPrivilegeTreeTest.java | 134 -----------
 5 files changed, 9 insertions(+), 513 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
index 333dd0e..1adfec8 100644
--- a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
@@ -20,24 +20,15 @@ package org.apache.impala.authorization.sentry;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Sets;
 import org.apache.impala.authorization.AuthorizationPolicy;
-import org.apache.impala.catalog.PrincipalPrivilege;
-import org.apache.impala.catalog.PrincipalPrivilegeTree;
 import org.apache.impala.catalog.Role;
 import org.apache.impala.catalog.User;
 import org.apache.sentry.core.common.ActiveRoleSet;
-import org.apache.sentry.core.common.Authorizable;
-import org.apache.sentry.core.common.utils.SentryConstants;
-import org.apache.sentry.core.model.db.DBModelAuthorizable;
-import org.apache.sentry.policy.common.Privilege;
-import org.apache.sentry.policy.common.PrivilegeFactory;
-import org.apache.sentry.policy.engine.common.CommonPrivilegeFactory;
-import org.apache.sentry.provider.cache.FilteredPrivilegeCache;
+import org.apache.sentry.provider.cache.PrivilegeCache;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 /**
  * The source data this cache is backing is read from the Sentry Policy Service.
@@ -46,17 +37,15 @@ import java.util.stream.Collectors;
  * TODO: Instead of calling into Sentry to perform final authorization checks, we
  * should parse/validate the privileges in Impala.
  */
-public class SentryAuthorizationPolicy implements FilteredPrivilegeCache {
+public class SentryAuthorizationPolicy implements PrivilegeCache {
   private static final Logger LOG = LoggerFactory.getLogger(
       SentryAuthorizationPolicy.class);
 
   private final AuthorizationPolicy authzPolicy_;
-  private final PrivilegeFactory privilegeFactory;
 
   public SentryAuthorizationPolicy(AuthorizationPolicy authzPolicy) {
     Preconditions.checkNotNull(authzPolicy);
     authzPolicy_ = authzPolicy;
-    this.privilegeFactory = new CommonPrivilegeFactory();
   }
 
   /**
@@ -65,12 +54,6 @@ public class SentryAuthorizationPolicy implements FilteredPrivilegeCache {
   @Override
   public Set<String> listPrivileges(Set<String> groups,
       ActiveRoleSet roleSet) {
-    return listPrivilegesForGroups(groups, roleSet, null);
-  }
-
-
-  private Set<String> listPrivilegesForGroups(Set<String> groups,
-      ActiveRoleSet roleSet, PrincipalPrivilegeTree.Filter filter) {
     Set<String> privileges = Sets.newHashSet();
     if (roleSet != ActiveRoleSet.ALL) {
       throw new UnsupportedOperationException("Impala does not support role subsets.");
@@ -80,7 +63,7 @@ public class SentryAuthorizationPolicy implements FilteredPrivilegeCache {
     for (String groupName: groups) {
       List<Role> grantedRoles = authzPolicy_.getGrantedRoles(groupName);
       for (Role role: grantedRoles) {
-        privileges.addAll(role.getFilteredPrivilegeNames(filter));
+        privileges.addAll(role.getPrivilegeNames());
       }
     }
     return privileges;
@@ -92,75 +75,17 @@ public class SentryAuthorizationPolicy implements FilteredPrivilegeCache {
   @Override
   public Set<String> listPrivileges(Set<String> groups, Set<String> users,
       ActiveRoleSet roleSet) {
-    return listPrivilegesForGroupsAndUsers(groups, users, roleSet, null);
-  }
-
-  private Set<String> listPrivilegesForGroupsAndUsers(Set<String> groups,
-      Set<String> users, ActiveRoleSet roleSet, PrincipalPrivilegeTree.Filter filter) {
-    Set<String> privileges = listPrivilegesForGroups(groups, roleSet, filter);
+    Set<String> privileges = listPrivileges(groups, roleSet);
     for (String userName: users) {
       User user = authzPolicy_.getUser(userName);
       if (user != null) {
-        privileges.addAll(user.getFilteredPrivilegeNames(filter));
+        privileges.addAll(user.getPrivilegeNames());
       }
     }
     return privileges;
   }
 
   @Override
-  public Set<String> listPrivileges(Set<String> groups, Set<String> users,
-      ActiveRoleSet roleSet, Authorizable... authorizationHierarchy) {
-    PrincipalPrivilegeTree.Filter filter = createPrivilegeFilter(authorizationHierarchy);
-    return listPrivilegesForGroupsAndUsers(groups, users, roleSet, filter);
-  }
-
-  @Override
-  public Set<Privilege> listPrivilegeObjects(Set<String> groups, Set<String> users,
-      ActiveRoleSet roleSet, Authorizable... authorizationHierarchy) {
-    Set<String> privilegeStrings =
-        listPrivileges(groups, users, roleSet, authorizationHierarchy);
-
-    return privilegeStrings.stream()
-      .filter(priString -> priString != null)
-      .map(priString -> getPrivilegeObject(priString))
-      .collect(Collectors.toSet());
-  }
-
-  private Privilege getPrivilegeObject(String priString) {
-    return privilegeFactory.createPrivilege(priString);
-  }
-
-  private PrincipalPrivilegeTree.Filter createPrivilegeFilter(
-      Authorizable... authorizationHierarchy) {
-    PrincipalPrivilegeTree.Filter filter = new PrincipalPrivilegeTree.Filter();
-    for (Authorizable auth : authorizationHierarchy) {
-      String name = auth.getName().toLowerCase();
-      if (name.equals(SentryConstants.RESOURCE_WILDCARD_VALUE) ||
-        name.equals(SentryConstants.RESOURCE_WILDCARD_VALUE_SOME)||
-        name.equals(SentryConstants.RESOURCE_WILDCARD_VALUE_ALL)) {
-        name = null; // null will match with everything
-      }
-      if (!(auth instanceof DBModelAuthorizable)) continue;
-      DBModelAuthorizable dbAuth = (DBModelAuthorizable) auth;
-      switch (dbAuth.getAuthzType()) {
-        case Server:
-          filter.setServer(name);
-          break;
-        case Db:
-          filter.setDb(name);
-          break;
-        case Table:
-          filter.setTable(name);
-          break;
-        case URI:
-          filter.setIsUri(true);
-        // Do not do anything for Column and View
-      }
-    }
-    return filter;
-  }
-
-  @Override
   public void close() {
     // Nothing to do, but required by PrivilegeCache.
   }
diff --git a/fe/src/main/java/org/apache/impala/catalog/Principal.java b/fe/src/main/java/org/apache/impala/catalog/Principal.java
index 6d03e9d..39aeef1 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Principal.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Principal.java
@@ -22,7 +22,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.impala.thrift.TCatalogObject;
 import org.apache.impala.thrift.TCatalogObjectType;
@@ -44,17 +43,6 @@ public abstract class Principal extends CatalogObjectImpl {
   private final CatalogObjectCache<PrincipalPrivilege> principalPrivileges_ =
       new CatalogObjectCache<>(false);
 
-  // An index that allows efficient filtering of Privileges that are relevant to an
-  // access check. Should contain exactly the same privileges as principalPrivileges_.
-  // Does not support catalog version logic / removal of privileges by privilegeName,
-  // so principalPrivileges_ is still needed.
-  private final PrincipalPrivilegeTree privilegeTree_ = new PrincipalPrivilegeTree();
-
-  // Protects privilegeTree_ and its coherence with principalPrivileges_.
-  // Needs to be taken when accessing privilegeTree_ or when writing principalPrivileges_,
-  // but not when reading principalPrivileges_.
-  private final ReentrantReadWriteLock rwLock_ = new ReentrantReadWriteLock(true);
-
   protected Principal(String principalName, TPrincipalType type,
       Set<String> grantGroups) {
     principal_ = new TPrincipal();
@@ -74,14 +62,7 @@ public abstract class Principal extends CatalogObjectImpl {
    * to the principal.
    */
   public boolean addPrivilege(PrincipalPrivilege privilege) {
-    try {
-      rwLock_.writeLock().lock();
-      if (!principalPrivileges_.add(privilege)) return false;
-      privilegeTree_.add(privilege);
-    } finally {
-      rwLock_.writeLock().unlock();
-    }
-    return true;
+    return principalPrivileges_.add(privilege);
   }
 
   /**
@@ -93,7 +74,7 @@ public abstract class Principal extends CatalogObjectImpl {
   }
 
   /**
-   * Returns all privilege names for this principal, or an empty set if no privileges are
+   * Returns all privilege names for this principal, or an empty set of no privileges are
    * granted to the principal.
    */
   public Set<String> getPrivilegeNames() {
@@ -101,25 +82,6 @@ public abstract class Principal extends CatalogObjectImpl {
   }
 
   /**
-   * Returns all privilege names for this principal that match 'filter'.
-   */
-  public Set<String> getFilteredPrivilegeNames(PrincipalPrivilegeTree.Filter filter) {
-    if (filter == null) return getPrivilegeNames();
-
-    List<PrincipalPrivilege> privileges;
-    try {
-      rwLock_.readLock().lock();
-      privileges = privilegeTree_.getFilteredList(filter);
-    } finally {
-      rwLock_.readLock().unlock();
-    }
-
-    Set<String> results = new HashSet<>();
-    for (PrincipalPrivilege priv: privileges) results.add(priv.getName());
-    return results;
-  }
-
-  /**
    * Gets a privilege with the given name from this principal. If no privilege exists
    * with this name null is returned.
    */
@@ -132,14 +94,7 @@ public abstract class Principal extends CatalogObjectImpl {
    * privilege or null if no privilege exists with this name.
    */
   public PrincipalPrivilege removePrivilege(String privilegeName) {
-    try {
-      rwLock_.writeLock().lock();
-      PrincipalPrivilege privilege = principalPrivileges_.remove(privilegeName);
-      if (privilege != null) privilegeTree_.remove(privilege);
-      return privilege;
-    } finally {
-      rwLock_.writeLock().unlock();
-    }
+    return principalPrivileges_.remove(privilegeName);
   }
 
   /**
diff --git a/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java b/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
index 7d1b90f..1b2da26 100644
--- a/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
+++ b/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
@@ -37,11 +37,9 @@ public class PrincipalPrivilege extends CatalogObjectImpl {
   private static final String AUTHORIZABLE_SEPARATOR = "->";
   private static final String KV_SEPARATOR = "=";
   private final TPrivilege privilege_;
-  private final String name_;
 
   private PrincipalPrivilege(TPrivilege privilege) {
     privilege_ = Preconditions.checkNotNull(privilege);
-    name_ =  buildPrivilegeName(privilege_);
   }
 
   public TPrivilege toThrift() { return privilege_; }
@@ -155,7 +153,7 @@ public class PrincipalPrivilege extends CatalogObjectImpl {
     return TCatalogObjectType.PRIVILEGE;
   }
   @Override
-  public String getName() { return name_; }
+  public String getName() { return buildPrivilegeName(privilege_); }
   public int getPrincipalId() { return privilege_.getPrincipal_id(); }
   public TPrincipalType getPrincipalType() { return privilege_.getPrincipal_type(); }
 
diff --git a/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilegeTree.java b/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilegeTree.java
deleted file mode 100644
index 61d87da..0000000
--- a/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilegeTree.java
+++ /dev/null
@@ -1,248 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package org.apache.impala.catalog;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import org.apache.impala.thrift.TPrivilege;
-import org.apache.impala.thrift.TPrivilegeScope;
-
-import com.google.common.base.Preconditions;
-
-/**
- * Tree that allows efficient lookup for all privileges that can be relevant in the
- * authorization of an object, e.g. getting privileges for server1/db1 should only iterate
- * through privileges in db1, but not in other databases.
- *
- * Performance characteristics:
- * add/remove: O(1), as the depth of the tree is limited
- * getFilteredList: O(number_of_privileges_that_match_the_filter)
- *
- * The primary motivation is to speed up SHOW DATABASES/TABLES calls which do a separate
- * listing to check access rights for each database/table. For this reason column and URI
- * privileges do not get their own "level" in the tree - column privileges are merged with
- * table privileges and URI privileges are stored per-server. This avoids unnecessarily
- * storing many small hash maps in memory.
- *
- * This class is expected to be used in pair with a CatalogObjectCache<PrincipalPrivilege>
- * which also handles catalog version logic and has efficient "remove" by full name.
- */
-public class PrincipalPrivilegeTree {
-   // Contains database/table/column privileges grouped by server name + database name
-   // + table name. Column privileges are stored with table privileges to avoid the
-   // memory cost of creating many small hashmaps for columns with privileges.
-   Node<PrincipalPrivilege> tableRoot_ = new Node<>();
-
-   // Contains URI privileges grouped by server name. Storing these separately from table
-   // privileges allows Node to be simpler, as the Server level doesn't need to
-   // differentiate between two kind of privileges.
-   Node<PrincipalPrivilege> uriRoot_ = new Node<>();
-
-   // Contains server privileges grouped by server name. Stored separately from other
-   // privileges, as these should be returned both when listing URI and non-URI
-   // privileges.
-   Node<PrincipalPrivilege> serverRoot_ = new Node<>();
-
-   public void add(PrincipalPrivilege privilege) {
-     TPrivilege priv = privilege.toThrift();
-     List<String> path = toPath(priv);
-     Node<PrincipalPrivilege> root = getRootForScope(priv.getScope());
-     root.add(privilege.getName(), privilege, path);
-   }
-
-   public void remove(PrincipalPrivilege privilege) {
-     TPrivilege priv = privilege.toThrift();
-     List<String> path = toPath(priv);
-     Node<PrincipalPrivilege> root = getRootForScope(priv.getScope());
-     root.remove(privilege.getName(), privilege, path);
-   }
-
-   /**
-   * Collect all privileges that match the filter.
-   * E.g. for server1.db1, it returns privileges on:
-   *  server1, server1.db1, server1.db1.table1,
-   * but not privileges on:
-   *  server2, server2.db1, server1.db2
-   */
-   public List<PrincipalPrivilege> getFilteredList(Filter filter) {
-     List<String> path = filter.toPath();
-     List<PrincipalPrivilege> results = new ArrayList<>();
-     List<Node<PrincipalPrivilege>> roots = new ArrayList<>();
-     // Server level privileges apply to both URIs and other objects.
-     roots.add(serverRoot_);
-     if (filter.isUri_ || path.size() == 1) roots.add(uriRoot_);
-     if (!filter.isUri_) roots.add(tableRoot_);
-     for (Node<PrincipalPrivilege> root : roots) {
-       root.getAllMatchingValues(results, path);
-     }
-     return results;
-   }
-
-   private Node<PrincipalPrivilege> getRootForScope(TPrivilegeScope scope) {
-     switch(scope) {
-     case URI: return uriRoot_;
-     case SERVER: return serverRoot_;
-     default: return tableRoot_;
-     }
-   }
-
-   /**
-    * Creates a path to the given privilege in the tree like ["server1", "db1", "table1"].
-    */
-   private static List<String> toPath(TPrivilege priv) {
-      List<String> path = new ArrayList<>();
-      String server = priv.getServer_name();
-      String db = priv.getDb_name();
-      String table = priv.getTable_name();
-      if (server == null) return path;
-      path.add(server.toLowerCase());
-      if (db == null) return path;
-      path.add(db.toLowerCase());
-      if (table != null) path.add(table.toLowerCase());
-      return path;
-   }
-
-  /**
-   * Lossy representation of an Authorizable (doesn't contain column/URI name).
-   * Can be used to rule out the bulk of the privileges that can have no effect on an
-   * access check.
-   */
-  public static class Filter {
-    String server_, db_, table_; // must be lower case, null matches everything
-    boolean isUri_ = false;
-
-    public void setServer(String server) { server_ = server; }
-    public void setDb(String db) { db_ = db; }
-    public void setTable(String  table) { table_ = table; }
-    public void setIsUri(boolean isUri) { isUri_ = isUri; }
-
-   /**
-    * Creates a path till the first null element of the filter, e.g.
-    * ["server1", "db1"] if table_ is null.
-    */
-    private List<String> toPath() {
-      List<String> path = new ArrayList<>();
-      if (server_ == null) return path;
-      path.add(server_);
-      if (db_ == null) return path;
-      Preconditions.checkState(!isUri_);
-      path.add(db_);
-      if (table_ != null) path.add(table_);
-      return path;
-    }
-  }
-
-  /**
-   * Tree node that holds the privileges for a given object (server, database, table),
-   * and its children objects (e.g. databases for a server). Descendants can be addressed
-   * with paths like ["server1", "db1", "table1"].
-   *
-   * Only used to store privileges, but creating a generic class seemed clearer.
-   */
-  private static class Node<T> {
-    Map<String, T> values_ = null;
-    Map<String, Node<T>> children_= null;
-
-    boolean isEmpty() { return values_ == null && children_ == null; }
-
-    /**
-     * Finds the Node at 'path' (or potentially builds it if it doesn't exist),
-     * and adds 'key' + 'value' to it.
-     */
-    public void add(String key, T value, List<String> path) {
-      add(key, value, path, 0);
-    }
-
-    /**
-     * Finds the Node at 'path' (it is treated as error if it doesn't exist),
-     * and removes 'key' + 'value' from it. If a Node becomes empty, it is removed from
-     * it's parent.
-     */
-    public void remove(String key, T value, List<String> path) {
-      remove(key, value, path, 0);
-    }
-
-    /**
-     * Collect all values in this node and those descendants that match 'path'.
-     */
-    public void getAllMatchingValues(List<T> results, List<String> path) {
-      getAllMatchingValues(results, path, 0);
-    }
-
-    /**
-     * Collect all values in this node and its descendants.
-     */
-    public void getAllValues(List<T> results) {
-      if (values_ != null) results.addAll(values_.values());
-      if (children_ != null) {
-        for (Map.Entry<String, Node<T>> entry : children_.entrySet()) {
-          entry.getValue().getAllValues(results);
-        }
-      }
-    }
-
-    private void add(String key, T value, List<String> path, int depth) {
-      if (path.size() <= depth) {
-        if (values_ == null) {
-          // It is very common to have only a single privilege on an object
-          // (e.g. ownership), so the initial capacity is 1 to avoid wasting memory.
-          values_ = new HashMap<>(1);
-        }
-        values_.put(key, value); // Can update an existing value.
-      } else {
-        if (children_ == null) children_ = new HashMap<>();
-        String child_name = path.get(depth);
-        Node<T> next = children_.computeIfAbsent(child_name, (k) -> new Node<T>());
-        next.add(key, value, path, depth + 1);
-      }
-    }
-
-    private void remove(String key, T value, List<String> path, int depth) {
-      if (path.size() <= depth) {
-        Preconditions.checkNotNull(values_);
-        T found = values_.remove(key);
-        Preconditions.checkNotNull(found);
-        if (values_.isEmpty()) values_ = null;
-      } else {
-        Preconditions.checkNotNull(children_);
-        String child_name = path.get(depth);
-        Node<T> next = children_.get(child_name);
-        Preconditions.checkNotNull(next);
-        next.remove(key, value, path, depth + 1);
-        // Remove the child if it became empty.
-        if (next.isEmpty()) children_.remove(child_name);
-        if (children_.isEmpty()) children_ = null;
-      }
-    }
-
-    private void getAllMatchingValues(List<T> results, List<String> path, int depth) {
-      if (path.size() <= depth) {
-        getAllValues(results);
-        return;
-      }
-      if (values_ != null) results.addAll(values_.values());
-      if (children_ == null) return;
-      String child_name = path.get(depth);
-      Preconditions.checkNotNull(child_name);
-      Node<T> next = children_.get(child_name);
-      if (next != null) next.getAllMatchingValues(results, path, depth + 1);
-    }
-  }
-}
diff --git a/fe/src/test/java/org/apache/impala/catalog/PrincipalPrivilegeTreeTest.java b/fe/src/test/java/org/apache/impala/catalog/PrincipalPrivilegeTreeTest.java
deleted file mode 100644
index dfc38f1..0000000
--- a/fe/src/test/java/org/apache/impala/catalog/PrincipalPrivilegeTreeTest.java
+++ /dev/null
@@ -1,134 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package org.apache.impala.catalog;
-
-import org.junit.Test;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertSame;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.impala.thrift.TPrivilege;
-import org.apache.impala.thrift.TPrivilegeLevel;
-import org.apache.impala.thrift.TPrivilegeScope;
-
-public class PrincipalPrivilegeTreeTest {
-  @Test
-  public void testPrincipalPrivilegeTree() {
-    // This test is mainly based on TestTreePrivilegeCache.testListPrivilegesWildCard in
-    // Sentry.
-    List<PrincipalPrivilege> privs = new ArrayList<>();
-
-    privs.add(createTablePriv("server1", "db1", "t1"));
-    privs.add(createTablePriv("server1", "db2", "t1"));
-    privs.add(createTablePriv("server1", "db1", "t2"));
-    privs.add(createDbPriv("server1", "db1"));
-    privs.add(createServerPriv("server1"));
-    privs.add(createServerPriv("server2"));
-    privs.add(createColumnPriv("server1", "db1", "t1", "c1"));
-    privs.add(createUriPriv("server1", "uri1"));
-
-    PrincipalPrivilegeTree tree = new PrincipalPrivilegeTree();
-    // Run the same tests twice to check if removing privileges works correctly.
-    for (int i = 0; i < 2; i++) {
-      for (PrincipalPrivilege priv : privs) tree.add(priv);
-
-      // Update a privilege and check that the newer object is returned by
-      // getFilteredList.
-      PrincipalPrivilege newServer2Priv = createServerPriv("server2");
-      tree.add(newServer2Priv);
-      List<PrincipalPrivilege> results =
-          tree.getFilteredList(createFilter("server2", null, null));
-      assertEquals(1, results.size());
-      assertSame(results.get(0), newServer2Priv);
-
-      assertEquals(7, tree.getFilteredList(createFilter("server1", null, null)).size());
-      assertEquals(5, tree.getFilteredList(createFilter("server1", "db1", null)).size());
-      assertEquals(2, tree.getFilteredList(createFilter("server1", "db2", null)).size());
-      assertEquals(4, tree.getFilteredList(createFilter("server1", "db1", "t1")).size());
-      assertEquals(2, tree.getFilteredList(createFilter("server1", "db2", "t1")).size());
-      assertEquals(2, tree.getFilteredList(createUriFilter("server1")).size());
-
-      // Check that all privileges are removed successfully.
-      for (PrincipalPrivilege priv : privs) tree.remove(priv);
-      assertEquals(0, tree.getFilteredList(createFilter("server1", null, null)).size());
-    }
-  }
-
-  PrincipalPrivilege createColumnPriv(String server, String db, String table,
-      String column) {
-    TPrivilege priv =
-        new TPrivilege(TPrivilegeLevel.SELECT, TPrivilegeScope.COLUMN, false);
-    priv.setServer_name(server);
-    priv.setDb_name(db);
-    priv.setTable_name(table);
-    priv.setColumn_name(column);
-    return PrincipalPrivilege.fromThrift(priv);
-  }
-
-  PrincipalPrivilege createTablePriv(String server, String db, String table) {
-    TPrivilege priv =
-        new TPrivilege(TPrivilegeLevel.SELECT, TPrivilegeScope.TABLE, false);
-    priv.setServer_name(server);
-    priv.setDb_name(db);
-    priv.setTable_name(table);
-    return PrincipalPrivilege.fromThrift(priv);
-  }
-
-  PrincipalPrivilege createDbPriv(String server, String db) {
-    TPrivilege priv =
-        new TPrivilege(TPrivilegeLevel.SELECT, TPrivilegeScope.DATABASE, false);
-    priv.setServer_name(server);
-    priv.setDb_name(db);
-    return PrincipalPrivilege.fromThrift(priv);
-  }
-
-  PrincipalPrivilege createUriPriv(String server, String uri) {
-    TPrivilege priv =
-        new TPrivilege(TPrivilegeLevel.SELECT, TPrivilegeScope.URI, false);
-    priv.setServer_name(server);
-    priv.setUri(uri);
-    return PrincipalPrivilege.fromThrift(priv);
-  }
-
-  PrincipalPrivilege createServerPriv(String server) {
-    TPrivilege priv =
-        new TPrivilege(TPrivilegeLevel.SELECT, TPrivilegeScope.SERVER, false);
-    priv.setServer_name(server);
-    return PrincipalPrivilege.fromThrift(priv);
-  }
-
-  PrincipalPrivilegeTree.Filter createFilter(String server, String db,
-      String table) {
-    PrincipalPrivilegeTree.Filter filter = new PrincipalPrivilegeTree.Filter();
-    filter.setServer(server);
-    filter.setDb(db);
-    filter.setTable(table);
-    filter.setIsUri(false);
-    return filter;
-  }
-
-  PrincipalPrivilegeTree.Filter createUriFilter(String server) {
-    PrincipalPrivilegeTree.Filter filter = new PrincipalPrivilegeTree.Filter();
-    filter.setServer(server);
-    filter.setIsUri(true);
-    return filter;
-  }
-}