You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by sa...@apache.org on 2018/04/05 02:58:21 UTC

[1/5] impala git commit: IMPALA-6457: [DOCS] DECIMAL support for Kudu tables

Repository: impala
Updated Branches:
  refs/heads/master 890bd4c72 -> 4d6b07f0e


IMPALA-6457: [DOCS] DECIMAL support for Kudu tables

Removed DECIMAL from the unsupported data types for Kudu.

Change-Id: I5a2498b4a28c2a53a3fec9b634a770e42dcac499
Reviewed-on: http://gerrit.cloudera.org:8080/9918
Reviewed-by: Grant Henke <gr...@apache.org>
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 7e8c9281d7e607b7db80a6670195b7861ab34005
Parents: 890bd4c
Author: Alex Rodoni <ar...@cloudera.com>
Authored: Tue Apr 3 15:04:20 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Apr 4 20:59:52 2018 +0000

----------------------------------------------------------------------
 docs/shared/impala_common.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/7e8c9281/docs/shared/impala_common.xml
----------------------------------------------------------------------
diff --git a/docs/shared/impala_common.xml b/docs/shared/impala_common.xml
index c272e4f..192e766 100644
--- a/docs/shared/impala_common.xml
+++ b/docs/shared/impala_common.xml
@@ -3917,7 +3917,7 @@ sudo pip-python install ssl</codeblock>
       </p>
 
       <p id="kudu_unsupported_data_type" rev="kudu">
-        Currently, the data types <codeph>DECIMAL</codeph>, <codeph>CHAR</codeph>, <codeph>VARCHAR</codeph>,
+        Currently, the data types <codeph>CHAR</codeph>, <codeph>VARCHAR</codeph>,
         <codeph>ARRAY</codeph>, <codeph>MAP</codeph>, and <codeph>STRUCT</codeph> cannot be used with Kudu tables.
       </p>
 


[5/5] impala git commit: IMPALA-6792: Fail status reporting if coordinator refuses connections

Posted by sa...@apache.org.
IMPALA-6792: Fail status reporting if coordinator refuses connections

The ReportExecStatusAux() function is run on a dedicated thread per
fragment instance. This thread will run until the fragment instance
completes executing.

On every attempt to send a report to the coordinator, it will attempt
to send up to 3 RPCs. If all 3 of them fail, then the fragment instance
will cancel itself.

However, there is one case where a failure to send the RPC will not
be considered a failed RPC. If when we attempt to obtain a new
connection, we end up creating a new connection
(via ClientCache::CreateClient()) instead of getting a previously
cached connection, and this new connection fails to even Open(),
it will not be counted as a RPC failure.

This patch counts such an error as a failed RPC too.

This patch also clarifies some of the error log messages and introduces
a flag to control the sleep interval between failed ReportExecStatus RPC
retries.

Change-Id: If668838f99f78b5ffa713488178b2eb5799ba220
Reviewed-on: http://gerrit.cloudera.org:8080/9916
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 4d6b07f0e2d18c8f5284d27b82bcee3aa9f1fe77
Parents: c9f4fdc
Author: Sailesh Mukil <sa...@cloudera.com>
Authored: Tue Apr 3 14:24:21 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu Apr 5 02:17:10 2018 +0000

----------------------------------------------------------------------
 be/src/runtime/query-state.cc | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/4d6b07f0/be/src/runtime/query-state.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/query-state.cc b/be/src/runtime/query-state.cc
index ad5748f..04a4283 100644
--- a/be/src/runtime/query-state.cc
+++ b/be/src/runtime/query-state.cc
@@ -38,6 +38,10 @@
 
 #include "common/names.h"
 
+DEFINE_int32(report_status_retry_interval_ms, 100,
+    "The interval in milliseconds to wait before retrying a failed status report RPC to "
+    "the coordinator.");
+
 using namespace impala;
 
 QueryState::ScopedRef::ScopedRef(const TUniqueId& query_id) {
@@ -249,28 +253,40 @@ void QueryState::ReportExecStatusAux(bool done, const Status& status,
   DCHECK_EQ(res.status.status_code, TErrorCode::OK);
   // Try to send the RPC 3 times before failing. Sleep for 100ms between retries.
   // It's safe to retry the RPC as the coordinator handles duplicate RPC messages.
+  Status client_status;
   for (int i = 0; i < 3; ++i) {
-    Status client_status;
     ImpalaBackendConnection client(ExecEnv::GetInstance()->impalad_client_cache(),
         query_ctx().coord_address, &client_status);
     if (client_status.ok()) {
       rpc_status = client.DoRpc(&ImpalaBackendClient::ReportExecStatus, params, &res);
       if (rpc_status.ok()) break;
     }
-    if (i < 2) SleepForMs(100);
+    if (i < 2) SleepForMs(FLAGS_report_status_retry_interval_ms);
   }
   Status result_status(res.status);
-  if ((!rpc_status.ok() || !result_status.ok()) && instances_started) {
+  if ((!client_status.ok() || !rpc_status.ok() || !result_status.ok()) &&
+      instances_started) {
     // TODO: should we try to keep rpc_status for the final report? (but the final
     // report, following this Cancel(), may not succeed anyway.)
     // TODO: not keeping an error status here means that all instances might
     // abort with CANCELLED status, despite there being an error
-    if (!rpc_status.ok()) {
-      // TODO: Fix IMPALA-2990. Cancelling fragment instances here may cause query to
-      // hang as the coordinator may not be aware of the cancellation. Remove the log
-      // statement once IMPALA-2990 is fixed.
-      LOG(ERROR) << "Cancelling fragment instances due to failure to report status. "
-                 << "Query " << PrintId(query_id()) << " may hang. See IMPALA-2990.";
+    // TODO: Fix IMPALA-2990. Cancelling fragment instances without sending the
+    // ReporExecStatus RPC may cause query to hang as the coordinator may not be aware
+    // of the cancellation. Remove the log statements once IMPALA-2990 is fixed.
+    if (!client_status.ok()) {
+      LOG(ERROR) << "Cancelling fragment instances due to failure to obtain a connection "
+                 << "to the coordinator. (" << client_status.GetDetail()
+                 << "). Query " << PrintId(query_id()) << " may hang. See IMPALA-2990.";
+    } else if (!rpc_status.ok()) {
+      LOG(ERROR) << "Cancelling fragment instances due to failure to reach the "
+                 << "coordinator. (" << rpc_status.GetDetail()
+                 << "). Query " << PrintId(query_id()) << " may hang. See IMPALA-2990.";
+    } else if (!result_status.ok()) {
+      // If the ReportExecStatus RPC succeeded in reaching the coordinator and we get
+      // back a non-OK status, it means that the coordinator expects us to cancel the
+      // fragment instances for this query.
+      LOG(INFO) << "Cancelling fragment instances as directed by the coordinator. "
+                << "Returned status: " << result_status.GetDetail();
     }
     Cancel();
   }


[4/5] impala git commit: IMPALA-6650: Add fine-grained DROP privilege

Posted by sa...@apache.org.
IMPALA-6650: Add fine-grained DROP privilege

Add support for executing DROP statements by granting DROP privilege.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT DROP on SERVER svr TO ROLE testrole;
GRANT DROP on DATABASE db TO ROLE testrole;
GRANT DROP on TABLE tbl TO ROLE testrole;

REVOKE DROP on SERVER svr FROM ROLE testrole;
REVOKE DROP on DATABASE db FROM ROLE testrole;
REVOKE DROP on TABLE tbl FROM ROLE testrole;

Testing:
- Added new front-end tests
- Ran front-end tests

Cherry-picks: not for 2.x

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


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

Branch: refs/heads/master
Commit: c9f4fdc6bc7145bb3fcfe88f15bbdf77656cdab5
Parents: 75e1bd1
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Tue Apr 3 10:58:23 2018 -0500
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Apr 4 22:05:20 2018 +0000

----------------------------------------------------------------------
 common/thrift/CatalogObjects.thrift             |   3 +-
 fe/.gitignore                                   |   3 +
 fe/src/main/cup/sql-parser.cup                  |   2 +
 .../impala/analysis/DropFunctionStmt.java       |   7 +-
 .../apache/impala/analysis/PrivilegeSpec.java   |   7 +-
 .../authorization/AuthorizationChecker.java     |   6 +-
 .../apache/impala/authorization/Privilege.java  |   3 +-
 .../impala/analysis/AnalyzeAuthStmtsTest.java   |  18 ++-
 .../impala/analysis/AuthorizationTest.java      | 128 +++++++++++++++++--
 .../org/apache/impala/analysis/ParserTest.java  |   6 +
 fe/src/test/resources/authz-policy.ini.template |   8 +-
 11 files changed, 159 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/common/thrift/CatalogObjects.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/CatalogObjects.thrift b/common/thrift/CatalogObjects.thrift
index 703e701..ecd27dc 100644
--- a/common/thrift/CatalogObjects.thrift
+++ b/common/thrift/CatalogObjects.thrift
@@ -470,7 +470,8 @@ enum TPrivilegeLevel {
   SELECT,
   REFRESH,
   CREATE,
-  ALTER
+  ALTER,
+  DROP
 }
 
 // Represents a privilege in an authorization policy. Privileges contain the level

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/.gitignore
----------------------------------------------------------------------
diff --git a/fe/.gitignore b/fe/.gitignore
index 6a1675d..11a5e9d 100644
--- a/fe/.gitignore
+++ b/fe/.gitignore
@@ -33,6 +33,9 @@ src/test/resources/authz-policy.ini
 # Generated minicluster config
 src/test/resources/minicluster-conf.xml
 
+# Generated hive-log4j2.properties file
+src/test/resources/hive-log4j2.properties
+
 # Generated thrift files
 generated-sources
 

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/main/cup/sql-parser.cup
----------------------------------------------------------------------
diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
index 4d6abfd..999ed16 100644
--- a/fe/src/main/cup/sql-parser.cup
+++ b/fe/src/main/cup/sql-parser.cup
@@ -966,6 +966,8 @@ privilege ::=
   {: RESULT = TPrivilegeLevel.CREATE; :}
   | KW_ALTER
   {: RESULT = TPrivilegeLevel.ALTER; :}
+  | KW_DROP
+  {: RESULT = TPrivilegeLevel.DROP; :}
   | KW_ALL
   {: RESULT = TPrivilegeLevel.ALL; :}
   ;

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java b/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
index b82b9d2..86e90c0 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
@@ -85,13 +85,10 @@ public class DropFunctionStmt extends StatementBase {
           false);
     }
 
-    // For now, if authorization is enabled, the user needs ALL on the server
-    // to drop functions.
-    // TODO: this is not the right granularity but acceptable for now.
     analyzer.registerPrivReq(new PrivilegeRequest(
-        new AuthorizeableFn(desc_.dbName(), desc_.signatureString()), Privilege.ALL));
+        new AuthorizeableFn(desc_.dbName(), desc_.signatureString()), Privilege.DROP));
 
-    Db db =  analyzer.getDb(desc_.dbName(), Privilege.DROP, false);
+    Db db =  analyzer.getDb(desc_.dbName(), false);
     if (db == null && !ifExists_) {
       throw new AnalysisException(Analyzer.DB_DOES_NOT_EXIST_ERROR_MSG + desc_.dbName());
     }

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
index b2652cc..ec292a2 100644
--- a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
+++ b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
@@ -191,9 +191,10 @@ public class PrivilegeSpec implements ParseNode {
         if (privilegeLevel_ != TPrivilegeLevel.ALL &&
             privilegeLevel_ != TPrivilegeLevel.REFRESH &&
             privilegeLevel_ != TPrivilegeLevel.CREATE &&
-            privilegeLevel_ != TPrivilegeLevel.ALTER) {
-          throw new AnalysisException("Only 'ALL', 'REFRESH', 'CREATE', or 'ALTER' " +
-              "privilege may be applied at SERVER scope in privilege spec.");
+            privilegeLevel_ != TPrivilegeLevel.ALTER &&
+            privilegeLevel_ != TPrivilegeLevel.DROP) {
+          throw new AnalysisException("Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or " +
+              "'DROP' privilege may be applied at SERVER scope in privilege spec.");
         }
         break;
       case DATABASE:

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
index 07bc4ad..9fcf020 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
@@ -105,13 +105,13 @@ public class AuthorizationChecker {
     Preconditions.checkNotNull(privilegeRequest);
 
     if (!hasAccess(user, privilegeRequest)) {
+      Privilege privilege = privilegeRequest.getPrivilege();
       if (privilegeRequest.getAuthorizeable() instanceof AuthorizeableFn) {
         throw new AuthorizationException(String.format(
-            "User '%s' does not have privileges to CREATE/DROP functions in: %s",
-            user.getName(), privilegeRequest.getName()));
+            "User '%s' does not have privileges to %s functions in: %s",
+            user.getName(), privilege, privilegeRequest.getName()));
       }
 
-      Privilege privilege = privilegeRequest.getPrivilege();
       if (EnumSet.of(Privilege.ANY, Privilege.ALL, Privilege.VIEW_METADATA)
           .contains(privilege)) {
         throw new AuthorizationException(String.format(

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/main/java/org/apache/impala/authorization/Privilege.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/authorization/Privilege.java b/fe/src/main/java/org/apache/impala/authorization/Privilege.java
index 558e4bd..59cc8d3 100644
--- a/fe/src/main/java/org/apache/impala/authorization/Privilege.java
+++ b/fe/src/main/java/org/apache/impala/authorization/Privilege.java
@@ -27,7 +27,7 @@ import org.apache.sentry.core.common.Action;
 public enum Privilege {
   ALL(SentryAction.ALL, false),
   ALTER(SentryAction.ALTER, false),
-  DROP(SentryAction.ALL, false),
+  DROP(SentryAction.DROP, false),
   CREATE(SentryAction.CREATE, false),
   INSERT(SentryAction.INSERT, false),
   SELECT(SentryAction.SELECT, false),
@@ -56,6 +56,7 @@ public enum Privilege {
     REFRESH("refresh"),
     CREATE("create"),
     ALTER("alter"),
+    DROP("drop"),
     ALL("*");
 
     private final String value;

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
index d1eae40..c56102f 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
@@ -165,8 +165,8 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest {
       AnalyzesOk(String.format("%s INSERT ON DATABASE functional %s myrole",
           formatArgs));
       AnalysisError(String.format("%s INSERT ON SERVER %s myrole", formatArgs),
-          "Only 'ALL', 'REFRESH', 'CREATE', or 'ALTER' privilege may be applied at " +
-          "SERVER scope in privilege spec.");
+          "Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or 'DROP' privilege may be " +
+          "applied at SERVER scope in privilege spec.");
       AnalysisError(String.format("%s INSERT ON URI 'hdfs:////abc//123' %s myrole",
           formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " +
           "spec.");
@@ -181,8 +181,8 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest {
       AnalyzesOk(String.format("%s SELECT ON DATABASE functional %s myrole",
           formatArgs));
       AnalysisError(String.format("%s SELECT ON SERVER %s myrole", formatArgs),
-          "Only 'ALL', 'REFRESH', 'CREATE', or 'ALTER' privilege may be applied at " +
-          "SERVER scope in privilege spec.");
+          "Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or 'DROP' privilege may be " +
+          "applied at SERVER scope in privilege spec.");
       AnalysisError(String.format("%s SELECT ON URI 'hdfs:////abc//123' %s myrole",
           formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " +
           "spec.");
@@ -255,6 +255,16 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest {
       AnalysisError(String.format(
           "%s ALTER ON URI 'hdfs:////abc/123' %s myrole", formatArgs),
           "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
+
+      // DROP privilege
+      AnalyzesOk(String.format("%s DROP ON SERVER %s myrole", formatArgs));
+      AnalyzesOk(String.format("%s DROP ON SERVER server1 %s myrole", formatArgs));
+      AnalyzesOk(String.format("%s DROP ON DATABASE functional %s myrole", formatArgs));
+      AnalyzesOk(String.format(
+          "%s DROP ON TABLE functional.alltypes %s myrole", formatArgs));
+      AnalysisError(String.format(
+          "%s DROP ON URI 'hdfs:////abc/123' %s myrole", formatArgs),
+          "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
     }
 
     AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx();

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
index 9085b78..a5173fb 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -40,6 +40,7 @@ import org.apache.impala.catalog.AuthorizationException;
 import org.apache.impala.catalog.Db;
 import org.apache.impala.catalog.ImpaladCatalog;
 import org.apache.impala.catalog.ScalarFunction;
+import org.apache.impala.catalog.ScalarType;
 import org.apache.impala.catalog.Type;
 import org.apache.impala.common.AnalysisException;
 import org.apache.impala.common.FrontendTestBase;
@@ -87,11 +88,11 @@ public class AuthorizationTest extends FrontendTestBase {
   //   ALL permission on 'tpch' database and 'newdb' database
   //   ALL permission on 'functional_seq_snap' database
   //   SELECT permissions on all tables in 'tpcds' database
-  //   SELECT, REFRESH permissions on 'functional.alltypesagg' (no INSERT permissions)
+  //   SELECT, REFRESH, DROP permissions on 'functional.alltypesagg'
   //   ALTER permissions on 'functional.alltypeserror'
-  //   SELECT permissions on 'functional.complex_view' (no INSERT permissions)
-  //   SELECT, REFRESH permissions on 'functional.view_view' (no INSERT permissions)
-  //   ALTER permission on 'functional.alltypes_view'
+  //   SELECT permissions on 'functional.complex_view'
+  //   SELECT, REFRESH permissions on 'functional.view_view'
+  //   ALTER, DROP permissions on 'functional.alltypes_view'
   //   SELECT permissions on columns ('id', 'int_col', and 'year') on
   //   'functional.alltypessmall' (no SELECT permissions on 'functional.alltypessmall')
   //   SELECT permissions on columns ('id', 'int_struct_col', 'struct_array_col',
@@ -105,7 +106,7 @@ public class AuthorizationTest extends FrontendTestBase {
   //   No permissions on database 'functional_rc'
   //   Only column level permissions in 'functional_avro':
   //     SELECT permissions on columns ('id') on 'functional_avro.alltypessmall'
-  //   REFRESH, INSERT, CREATE, ALTER permissions on 'functional_text_lzo' database
+  //   REFRESH, INSERT, CREATE, ALTER, DROP permissions on 'functional_text_lzo' database
   public final static String AUTHZ_POLICY_FILE = "/test-warehouse/authz-policy.ini";
   public final static User USER = new User(System.getProperty("user.name"));
 
@@ -267,6 +268,18 @@ public class AuthorizationTest extends FrontendTestBase {
     privilege.setTable_name("alltypesagg");
     sentryService.grantRolePrivilege(USER, roleName, privilege);
 
+    // drop_functional_alltypesagg
+    roleName = "drop_functional_alltypesagg";
+    sentryService.createRole(USER, roleName, true);
+    sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+
+    privilege = new TPrivilege("", TPrivilegeLevel.DROP,
+        TPrivilegeScope.TABLE, false);
+    privilege.setServer_name("server1");
+    privilege.setDb_name("functional");
+    privilege.setTable_name("alltypesagg");
+    sentryService.grantRolePrivilege(USER, roleName, privilege);
+
     // refresh_functional_view_view
     roleName = "refresh_functional_view_view";
     sentryService.createRole(USER, roleName, true);
@@ -279,6 +292,18 @@ public class AuthorizationTest extends FrontendTestBase {
     privilege.setTable_name("view_view");
     sentryService.grantRolePrivilege(USER, roleName, privilege);
 
+    // drop_functional_alltypes_view
+    roleName = "drop_functional_alltypes_view";
+    sentryService.createRole(USER, roleName, true);
+    sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+
+    privilege = new TPrivilege("", TPrivilegeLevel.DROP,
+        TPrivilegeScope.TABLE, false);
+    privilege.setServer_name("server1");
+    privilege.setDb_name("functional");
+    privilege.setTable_name("alltypes_view");
+    sentryService.grantRolePrivilege(USER, roleName, privilege);
+
     // insert_functional_text_lzo
     roleName = "insert_functional_text_lzo";
     sentryService.createRole(USER, roleName, true);
@@ -315,6 +340,18 @@ public class AuthorizationTest extends FrontendTestBase {
     privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
     sentryService.grantRolePrivilege(USER, roleName, privilege);
 
+    // drop_functional_text_lzo
+    roleName = "drop_functional_text_lzo";
+    sentryService.createRole(USER, roleName, true);
+    sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+
+    privilege = new TPrivilege("", TPrivilegeLevel.DROP, TPrivilegeScope.DATABASE,
+        false);
+    privilege.setServer_name("server1");
+    privilege.setDb_name("functional_text_lzo");
+    privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+    sentryService.grantRolePrivilege(USER, roleName, privilege);
+
     // alter_functional_alltypeserror
     roleName = "alter_functional_alltypeserror";
     sentryService.createRole(USER, roleName, true);
@@ -1244,6 +1281,15 @@ public class AuthorizationTest extends FrontendTestBase {
     AuthzOk("drop database if exists newdb");
     AuthzOk("drop database if exists newdb cascade");
     AuthzOk("drop database if exists newdb restrict");
+
+    // User has DROP privilege on functional_text_lzo database.
+    AuthzOk("drop database functional_text_lzo");
+    AuthzOk("drop database functional_text_lzo cascade");
+    AuthzOk("drop database functional_text_lzo restrict");
+    AuthzOk("drop database if exists functional_text_lzo");
+    AuthzOk("drop database if exists functional_text_lzo cascade");
+    AuthzOk("drop database if exists functional_text_lzo restrict");
+
     // User has permission, database does not exists, IF EXISTS not specified.
     try {
       AuthzOk("drop database newdb");
@@ -1294,6 +1340,9 @@ public class AuthorizationTest extends FrontendTestBase {
     AuthzOk("drop table tpch.lineitem");
     AuthzOk("drop table if exists tpch.lineitem");
 
+    // User has DROP privilege on functional.alltypesagg table.
+    AuthzOk("drop table functional.alltypesagg");
+
     // Drop table (user does not have permission).
     AuthzError("drop table functional.alltypes",
         "User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
@@ -1330,10 +1379,13 @@ public class AuthorizationTest extends FrontendTestBase {
     AuthzOk("drop view functional_seq_snap.alltypes_view");
     AuthzOk("drop view if exists functional_seq_snap.alltypes_view");
 
-    // Drop view (user does not have permission).
-    AuthzError("drop view functional.alltypes_view",
+    // User has DROP privilege on functional.alltypes_view view.
+    AuthzOk("drop view functional.alltypes_view");
+
+    // User does not have DROP privilege on functional.alltypes_view_sub view.
+    AuthzError("drop view functional.alltypes_view_sub",
         "User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
-    AuthzError("drop view if exists functional.alltypes_view",
+    AuthzError("drop view if exists functional.alltypes_view_sub",
         "User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
 
     // Drop view with unqualified table name.
@@ -2412,7 +2464,7 @@ public class AuthorizationTest extends FrontendTestBase {
 
     AuthzError(ctx, "create function f() returns int location " +
         "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
-        "User '%s' does not have privileges to CREATE/DROP functions in: default.f()");
+        "User '%s' does not have privileges to CREATE functions in: default.f()");
 
     // User has ALL privilege on tpch database and ALL privilege on
     // /test-warehouse/libTestUdfs.so URI.
@@ -2421,19 +2473,31 @@ public class AuthorizationTest extends FrontendTestBase {
 
     AuthzError(ctx, "create function notdb.f() returns int location " +
         "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
-        "User '%s' does not have privileges to CREATE/DROP functions in: notdb.f()");
+        "User '%s' does not have privileges to CREATE functions in: notdb.f()");
+
+    // User has DROP privilege on functional_text_lzo database.
+    try {
+      ctx_.catalog.addFunction(ScalarFunction.createForTesting("functional_text_lzo",
+          "f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
+          null, TFunctionBinaryType.NATIVE));
+      AuthzOk("drop function functional_text_lzo.f()");
+    } finally {
+      ctx_.catalog.removeFunction(ScalarFunction.createForTesting("functional_text_lzo",
+          "f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
+          null, TFunctionBinaryType.NATIVE));
+    }
 
     AuthzError(ctx, "drop function if exists f()",
-        "User '%s' does not have privileges to CREATE/DROP functions in: default.f()");
+        "User '%s' does not have privileges to DROP functions in: default.f()");
 
     AuthzError(ctx, "drop function notdb.f()",
-        "User '%s' does not have privileges to CREATE/DROP functions in: notdb.f()");
+        "User '%s' does not have privileges to DROP functions in: notdb.f()");
 
     // User does not have ALL privilege on SERVER and tries to create a function with
     // the same name as the built-in function.
     AuthzError(ctx, "create function sin(double) returns double location " +
         "'/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
-        "User '%s' does not have privileges to CREATE/DROP functions in: " +
+        "User '%s' does not have privileges to CREATE functions in: " +
         "default.sin(DOUBLE)");
     // User tries to create a function in the system database.
     AuthzError(ctx, "create function _impala_builtins.sin(double) returns double " +
@@ -2446,7 +2510,7 @@ public class AuthorizationTest extends FrontendTestBase {
     AuthzError(ctx, "drop function _impala_builtins.sin(double)",
         "Cannot modify system database.");
     AuthzError(ctx, "drop function sin(double)",
-        "User '%s' does not have privileges to CREATE/DROP functions in: " +
+        "User '%s' does not have privileges to DROP functions in: " +
         "default.sin(DOUBLE)");
 
     // TODO: Add test support for dynamically changing privileges for
@@ -2737,6 +2801,42 @@ public class AuthorizationTest extends FrontendTestBase {
     }
   }
 
+  @Test
+  public void TestServerLevelDrop() throws ImpalaException {
+    // TODO: Add test support for dynamically changing privileges for
+    // file-based policy.
+    if (ctx_.authzConfig.isFileBasedPolicy()) return;
+
+    SentryPolicyService sentryService =
+        new SentryPolicyService(ctx_.authzConfig.getSentryConfig());
+
+    // User has DROP privilege on server.
+    String roleName = "drop_role";
+    try {
+      sentryService.createRole(USER, roleName, true);
+      TPrivilege privilege = new TPrivilege("", TPrivilegeLevel.DROP,
+          TPrivilegeScope.SERVER, false);
+      privilege.setServer_name("server1");
+      sentryService.grantRolePrivilege(USER, roleName, privilege);
+      sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+      ctx_.catalog.reset();
+
+      AuthzOk("drop database functional");
+      AuthzOk("drop table functional.alltypes");
+      AuthzOk("drop view functional.alltypes_view_sub");
+      ctx_.catalog.addFunction(ScalarFunction.createForTesting("functional",
+          "f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
+          null, TFunctionBinaryType.NATIVE));
+      AuthzOk("drop function functional.f()");
+    } finally {
+      sentryService.dropRole(USER, roleName, true);
+      ctx_.catalog.reset();
+      ctx_.catalog.addFunction(ScalarFunction.createForTesting("functional",
+          "f", new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
+          null, TFunctionBinaryType.NATIVE));
+    }
+  }
+
   private void TestWithIncorrectConfig(AuthorizationConfig authzConfig, User user)
       throws ImpalaException {
     Frontend fe = new Frontend(authzConfig, ctx_.catalog);

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
index 3eab55e..ab0eb78 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
@@ -3592,6 +3592,12 @@ public class ParserTest extends FrontendTestBase {
       ParsesOk(String.format("%s ALTER ON DATABASE foo %s myRole", formatStr));
       ParsesOk(String.format("%s ALTER ON TABLE foo %s myRole", formatStr));
 
+      // DROP privilege.
+      ParsesOk(String.format("%s DROP ON SERVER %s myRole", formatStr));
+      ParsesOk(String.format("%s DROP ON SERVER foo %s myRole", formatStr));
+      ParsesOk(String.format("%s DROP ON DATABASE foo %s myRole", formatStr));
+      ParsesOk(String.format("%s DROP ON TABLE foo %s myRole", formatStr));
+
       // Server scope does not accept a name.
       ParsesOk(String.format("%s ALL ON SERVER %s myRole", formatStr));
       ParsesOk(String.format("%s INSERT ON SERVER %s myRole", formatStr));

http://git-wip-us.apache.org/repos/asf/impala/blob/c9f4fdc6/fe/src/test/resources/authz-policy.ini.template
----------------------------------------------------------------------
diff --git a/fe/src/test/resources/authz-policy.ini.template b/fe/src/test/resources/authz-policy.ini.template
index 82b1060..26ed70b 100644
--- a/fe/src/test/resources/authz-policy.ini.template
+++ b/fe/src/test/resources/authz-policy.ini.template
@@ -29,7 +29,8 @@ ${USER} = all_tpch, all_newdb, all_functional_seq_snap, select_tpcds,\
           refresh_functional_view_view, insert_functional_text_lzo,\
           create_functional_text_lzo, alter_functional_text_lzo,\
           alter_functional_alltypeserror, alter_functional_alltypes_view,\
-          libtestudfs_uri
+          libtestudfs_uri, drop_functional_text_lzo, drop_functional_alltypesagg,\
+          drop_functional_alltypes_view
 auth_to_local_group = test_role
 server_admin = all_server
 
@@ -55,15 +56,20 @@ insert_parquet = server=server1->db=functional_parquet->table=*->action=insert
 refresh_functional_text_lzo = server=server1->db=functional_text_lzo->action=refresh
 refresh_functional_alltypesagg =\
     server=server1->db=functional->table=alltypesagg->action=refresh
+drop_functional_alltypesagg =\
+    server=server1->db=functional->table=alltypesagg->action=drop
 alter_functional_alltypeserror =\
     server=server1->db=functional->table=alltypeserror->action=alter
 refresh_functional_view_view =\
     server=server1->db=functional->table=view_view->action=refresh
+drop_functional_alltypes_view =\
+    server=server1->db=functional->table=alltypes_view->action=drop
 alter_functional_alltypes_view =\
     server=server1->db=functional->table=alltypes_view->action=alter
 insert_functional_text_lzo = server=server1->db=functional_text_lzo->action=insert
 create_functional_text_lzo = server=server1->db=functional_text_lzo->action=create
 alter_functionl_text_lzo = server=server1->db=functional_text_lzo->action=alter
+drop_functional_text_lzo = server=server1->db=functional_text_lzo->action=drop
 select_column_level_functional =\
     server=server1->db=functional->table=alltypessmall->column=id->action=select,\
     server=server1->db=functional->table=alltypessmall->column=int_col->action=select,\


[3/5] impala git commit: IMPALA-6771: Fix in-predicate set up bug

Posted by sa...@apache.org.
IMPALA-6771: Fix in-predicate set up bug

Fixes a bug that introduced default initialized values in the set data
structure used to check for set membership that can cause wrong results.

Testing:
Added a test case that checks for the same.

Change-Id: I7e776dbcb7ee4a9b64e1295134a27d332f5415b6
Reviewed-on: http://gerrit.cloudera.org:8080/9891
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 75e1bd1bcd001770511247f2a762d9e74f38ce4f
Parents: b80a75f
Author: Bikramjeet Vig <bi...@cloudera.com>
Authored: Mon Apr 2 13:55:48 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Apr 4 21:51:29 2018 +0000

----------------------------------------------------------------------
 be/src/exprs/in-predicate.h                             |  3 ++-
 .../functional-query/queries/QueryTest/exprs.test       | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/75e1bd1b/be/src/exprs/in-predicate.h
----------------------------------------------------------------------
diff --git a/be/src/exprs/in-predicate.h b/be/src/exprs/in-predicate.h
index 2d439ba..87ae8b8 100644
--- a/be/src/exprs/in-predicate.h
+++ b/be/src/exprs/in-predicate.h
@@ -350,7 +350,8 @@ void InPredicate::SetLookupPrepare(
   state->contains_null = false;
   // Collect all values in a vector to use the bulk insert API to avoid N^2 behavior
   // with flat_set.
-  vector<SetType> element_list(ctx->GetNumArgs());
+  std::vector<SetType> element_list;
+  element_list.reserve(ctx->GetNumArgs() - 1);
   for (int i = 1; i < ctx->GetNumArgs(); ++i) {
     DCHECK(ctx->IsArgConstant(i));
     T* arg = reinterpret_cast<T*>(ctx->GetConstantArg(i));

http://git-wip-us.apache.org/repos/asf/impala/blob/75e1bd1b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/exprs.test b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
index 57ff3c6..1ac96f0 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/exprs.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/exprs.test
@@ -2995,3 +2995,15 @@ select cast('2001-1-2' as timestamp)
 ---- TYPES
 timestamp
 ====
+---- QUERY
+# IMPALA-6771: Test that the in-predicate set does not have default initialized
+# values that can result in wrong results. For a string column the default initialized
+# value is an empty string.
+select count(*) from functional.alltypes
+where regexp_replace(string_col, '1', '')
+in ('0', '1', '2', '3', '4', '5', '6', '7', '8', '9')
+---- RESULTS
+6570
+---- TYPES
+bigint
+====


[2/5] impala git commit: IMPALA-6667: [DOCS] max_cached_file_handles is enabled by default

Posted by sa...@apache.org.
IMPALA-6667: [DOCS] max_cached_file_handles is enabled by default

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


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

Branch: refs/heads/master
Commit: b80a75f07bd0b6b37d32b83f559a6794033c5c2e
Parents: 7e8c928
Author: Alex Rodoni <ar...@cloudera.com>
Authored: Tue Apr 3 14:49:28 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Apr 4 21:26:25 2018 +0000

----------------------------------------------------------------------
 docs/topics/impala_scalability.xml | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/b80a75f0/docs/topics/impala_scalability.xml
----------------------------------------------------------------------
diff --git a/docs/topics/impala_scalability.xml b/docs/topics/impala_scalability.xml
index beb450f..22e8e72 100644
--- a/docs/topics/impala_scalability.xml
+++ b/docs/topics/impala_scalability.xml
@@ -977,7 +977,7 @@ so other secure services might be affected temporarily.
   <title>Kerberos-Related Memory Overhead for Large Clusters</title>
   <conbody>
     <p conref="../shared/impala_common.xml#common/vm_overcommit_memory_intro"/>
-    <p conref="../shared/impala_common.xml#common/vm_overcommit_memory_start" conrefend="vm_overcommit_memory_end"/>
+    <p conref="../shared/impala_common.xml#common/vm_overcommit_memory_start" conrefend="../shared/impala_common.xml#common/vm_overcommit_memory_end"/>
   </conbody>
   </concept>
 
@@ -1032,26 +1032,26 @@ so other secure services might be affected temporarily.
         (that is, increase latency) to Impala queries, and reduce overall throughput for non-Impala
         workloads that also require accessing HDFS files.
       </p>
-      <p>
-        In <keyword keyref="impala210_full"/> and higher, you can reduce NameNode overhead by enabling
-        a caching feature for HDFS file handles. Data files that are accessed by different queries,
-        or even multiple times within the same query, can be accessed without a new <q>open</q>
-        call and without fetching the file details again from the NameNode.
-      </p>
+      <p> In <keyword keyref="impala210_full"/> and higher, you can reduce
+        NameNode overhead by enabling a caching feature for HDFS file handles.
+        Data files that are accessed by different queries, or even multiple
+        times within the same query, can be accessed without a new <q>open</q>
+        call and without fetching the file details again from the NameNode. </p>
       <p>
         Because this feature only involves HDFS data files, it does not apply to non-HDFS tables,
         such as Kudu or HBase tables, or tables that store their data on cloud services such as
         S3 or ADLS. Any read operations that perform remote reads also skip the cached file handles.
       </p>
-      <p>
-        This feature is turned off by default. To enable it, set the configuration option
-        <codeph>max_cached_file_handles</codeph> to a non-zero value for each <cmdname>impalad</cmdname>
-        daemon. Consider an initial starting value of 20 thousand, and adjust upward if NameNode
-        overhead is still significant, or downward if it is more important to reduce the extra memory usage
-        on each host. Each cache entry consumes 6 KB, meaning that caching 20,000 file handles requires
-        up to 120 MB on each DataNode. The exact memory usage varies depending on how many file handles
-        have actually been cached; memory is freed as file handles are evicted from the cache.
-      </p>
+      <p> The feature is enabled by default with 20,000 file handles to be
+        cached. To change the value, set the configuration option
+          <codeph>max_cached_file_handles</codeph> to a non-zero value for each
+          <cmdname>impalad</cmdname> daemon. From the initial default value of
+        20000, adjust upward if NameNode request load is still significant, or
+        downward if it is more important to reduce the extra memory usage on
+        each host. Each cache entry consumes 6 KB, meaning that caching 20,000
+        file handles requires up to 120 MB on each Impala executor. The exact
+        memory usage varies depending on how many file handles have actually
+        been cached; memory is freed as file handles are evicted from the cache. </p>
       <p>
         If a manual HDFS operation moves a file to the HDFS Trashcan while the file handle is cached,
         Impala still accesses the contents of that file. This is a change from prior behavior. Previously,