[impala] 04/04: IMPALA-9625: Convert the name of a TAccessEvent to lowercase

[ta...@apache.org]

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

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

commit e93405f7600c2cefcf522fdc64162a9ceaac3aa3
Author: Fang-Yu Rao <fa...@cloudera.com>
AuthorDate: Wed Apr 8 12:05:25 2020 -0700

    IMPALA-9625: Convert the name of a TAccessEvent to lowercase
    
    Impala's COMPUTE STATS statement results in two registrations of the
    ALTER event for the corresponding table identified by its
    fully-qualified table name and a Set is used to maintained the audits.
    The first registration is in Analyzer#registerAuthAndAuditEvent() and
    the second is in Analyzer#getTable().
    
    In registerAuthAndAuditEvent(), the corresponding full table name
    table.getFullName() is produced by a call to Analyzer#resolveTableRef().
    The resulting database and table names are both in lowercase.
    
    However, in getTable(), the fully-qualified table name is produced by a
    call to Analyzer#getFqTableName(). The resulting database and table
    names are in their originally unconverted form provided by the user from
    the Impala shell. Hence, there is no guarantee that the database and
    table names are both in lowercase.
    
    Therefore, if a user does not provide lowercase database and table
    names, the returned full table names from registerAuthAndAuditEvent()
    and getTable() would differ, resulting in duplicate ALTER events for the
    same table. This patch resolves the inconsistencies by converting the
    name of a TAccessEvent to lowercase in addAccessEvent(), which is called
    in getTable().
    
    Testing:
    - Revised a FE test to verified we do not have duplicate ALTER events for
      the COMPUTE STATS statement.
    - Verified that the patch passes the exhaustive tests in the DEBUG build
      except for a flaky E2E test of test_column_storage_attributes.
    
    Change-Id: If0d9ba58da891921fafbfe7c6db358b51965e178
    Reviewed-on: http://gerrit.cloudera.org:8080/15689
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/analysis/Analyzer.java    | 12 ++++++++----
 .../test/java/org/apache/impala/analysis/AuditingTest.java   | 10 ++++++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index 91de1bc..367edab 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -2756,6 +2756,10 @@ public class Analyzer {
    */
   public Set<TAccessEvent> getAccessEvents() { return globalState_.accessEvents; }
   public void addAccessEvent(TAccessEvent event) {
+    // We convert 'event.name' to lowercase to avoid duplicate access events since a
+    // statement could possibly result in two calls to addAccessEvent(), e.g.,
+    // COMPUTE STATS.
+    event.name = event.name.toLowerCase();
     globalState_.accessEvents.add(event);
   }
 
@@ -2859,8 +2863,8 @@ public class Analyzer {
       TCatalogObjectType objectType = TCatalogObjectType.TABLE;
       if (table instanceof FeView) objectType = TCatalogObjectType.VIEW;
       for (Privilege priv : privilege) {
-        globalState_.accessEvents.add(new TAccessEvent(
-            fqTableName.toString(), objectType, priv.toString()));
+        addAccessEvent(new TAccessEvent(fqTableName.toString(), objectType,
+            priv.toString()));
       }
     }
     return table;
@@ -2943,8 +2947,8 @@ public class Analyzer {
     });
     // Propagate the exception if needed.
     FeDb retDb = getDb(dbName, throwIfDoesNotExist);
-    globalState_.accessEvents.add(new TAccessEvent(
-        dbName, TCatalogObjectType.DATABASE, privilege.toString()));
+    addAccessEvent(new TAccessEvent(dbName, TCatalogObjectType.DATABASE,
+        privilege.toString()));
     return retDb;
   }
 
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
index c4278ab..4310c50 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
@@ -300,6 +300,16 @@ public class AuditingTest extends FrontendTestBase {
             "functional_seq_snap.alltypes", TCatalogObjectType.TABLE, "ALTER"),
         new TAccessEvent(
             "functional_seq_snap.alltypes", TCatalogObjectType.TABLE, "SELECT")));
+
+    // COMPUTE STATS results in two registrations of the ALTER event. Check we do not have
+    // duplicate ALTER events when the fully-qualified table name is not in lowercase.
+    accessEvents = AnalyzeAccessEvents(
+        "COMPUTE STATS FUNCTIONAL_SEQ_SNAP.ALLTYPES");
+    Assert.assertEquals(accessEvents, Sets.newHashSet(
+        new TAccessEvent(
+            "functional_seq_snap.alltypes", TCatalogObjectType.TABLE, "ALTER"),
+        new TAccessEvent(
+            "functional_seq_snap.alltypes", TCatalogObjectType.TABLE, "SELECT")));
   }
 
   @Test