You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/02/24 19:13:09 UTC

[2/3] impala git commit: IMPALA-6567: ResetMetadataStmt analysis should not load tables.

IMPALA-6567: ResetMetadataStmt analysis should not load tables.

This fixes a regression introduced by IMPALA-5152 where
invalidate metadata <tbl> and refresh <tbl> accidentally
required the target table to be loaded during analysis,
ultimately leading to a double load in some situations
(load during analysis, then another load during execution).
Since the purpose of these statements is to reload
metadata it does not make sense to require a table load
during analysis - that load happens during execution.

Note that REFRESH <tbl> PARTITION (<partition>) still
requires the containing table to be loaded. This was
the behavior before IMPALA-5152 and this patch does
not attempt to improve that.

Testing:
- added new unit test
- ran FE tests locally
- validated the desired behavior by inspecting logs
  and the timeine from invalidate/refresh statements

Change-Id: I7033781ebf27ea53cfd26ff0e4f74d4f242bd1dc
Reviewed-on: http://gerrit.cloudera.org:8080/9418
Tested-by: Impala Public Jenkins
Reviewed-by: Alex Behm <al...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/18902f0e
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/18902f0e
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/18902f0e

Branch: refs/heads/2.x
Commit: 18902f0ea30d757b6e11795ff8bfbb3aade769cc
Parents: 1795aca
Author: Alex Behm <al...@cloudera.com>
Authored: Thu Feb 22 21:07:27 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Sat Feb 24 01:59:35 2018 +0000

----------------------------------------------------------------------
 .../impala/analysis/ResetMetadataStmt.java      |  5 ++++-
 .../impala/analysis/StmtMetadataLoaderTest.java | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/18902f0e/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java b/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
index 1fd1e7c..e070d51 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
@@ -81,7 +81,10 @@ public class ResetMetadataStmt extends StatementBase {
 
   @Override
   public void collectTableRefs(List<TableRef> tblRefs) {
-    if (tableName_ != null) tblRefs.add(new TableRef(tableName_.toPath(), null));
+    // Only need table metadata for REFRESH <tbl> PARTITION (<partition>)
+    if (tableName_ != null && partitionSpec_ != null) {
+      tblRefs.add(new TableRef(tableName_.toPath(), null));
+    }
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/impala/blob/18902f0e/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
index f2c8faa..39416f8 100644
--- a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
@@ -47,6 +47,13 @@ public class StmtMetadataLoaderTest {
     validateCached(stmt, fe, expectedDbs, expectedTables);
   }
 
+  private void testNoLoad(String stmtStr) throws ImpalaException {
+    ImpaladTestCatalog catalog = new ImpaladTestCatalog();
+    Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
+    StatementBase stmt = fe.parse(stmtStr);
+    validateCached(stmt, fe, new String[]{}, new String[]{});
+  }
+
   private void validateDbs(StmtTableCache stmtTableCache, String[] expectedDbs) {
     String[] actualDbs = new String[stmtTableCache.dbs.size()];
     actualDbs = stmtTableCache.dbs.toArray(actualDbs);
@@ -177,4 +184,17 @@ public class StmtMetadataLoaderTest {
         new String[] {"functional.view_view", "functional.alltypes_view",
             "functional.alltypes"});
   }
+
+  @Test
+  public void testResetMetadataStmts() throws ImpalaException {
+    // These stmts should not request any table loads.
+    testNoLoad("invalidate metadata");
+    testNoLoad("invalidate metadata functional.alltypes");
+    testNoLoad("refresh functional.alltypes");
+    testNoLoad("refresh functions functional");
+
+    // This stmt requires the table to be loaded.
+    testLoadTables("refresh functional.alltypes partition (year=2009, month=1)", 1, 1,
+        new String[] {"default", "functional"}, new String[] {"functional.alltypes"});
+  }
 }