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 2019/06/13 06:37:17 UTC
[impala] 03/07: IMPALA-8649: Fix confusing SHOW GRANT error messages
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 d9f17c963407bfcb19c89dcaad6e9be17522cd1e
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Tue Jun 11 15:00:31 2019 -0700
IMPALA-8649: Fix confusing SHOW GRANT error messages
Before this patch, PrivilegeSpec had error messages tailored to
GRANT and REVOKE statements. However, PrivilegeSpec is also used in SHOW
GRANT statement. This patch updates the error messages to be less
confusing in SHOW GRANT statement.
Testing:
- Added new tests in AnalyzeAuthStmtsTest
- Ran AnalyzeAuthStmtsTest
Change-Id: Ibb88bdc19cd1223902b44e3634f756d086332266
Reviewed-on: http://gerrit.cloudera.org:8080/13587
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../org/apache/impala/analysis/PrivilegeSpec.java | 14 ++++----
.../impala/analysis/AnalyzeAuthStmtsTest.java | 37 +++++++++++++++-------
2 files changed, 33 insertions(+), 18 deletions(-)
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 e03c850..41a96a0 100644
--- a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
+++ b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
@@ -201,9 +201,9 @@ public class PrivilegeSpec extends StmtNode {
try {
analyzer.getDb(dbName_, true);
} catch (AnalysisException e) {
- throw new AnalysisException(String.format("Error setting privileges for " +
- "database '%s'. Verify that the database exists and that you have " +
- "permissions to issue a GRANT/REVOKE statement.", dbName_));
+ throw new AnalysisException(String.format("Error setting/showing privileges " +
+ "for database '%s'. Verify that the database exists and that you have " +
+ "permissions to issue a GRANT/REVOKE/SHOW GRANT statement.", dbName_));
}
break;
case URI:
@@ -252,9 +252,9 @@ public class PrivilegeSpec extends StmtNode {
for (String columnName: columnNames_) {
if (table.getColumn(columnName) == null) {
// The error message should not reveal the existence or absence of a column.
- throw new AnalysisException(String.format("Error setting column-level " +
+ throw new AnalysisException(String.format("Error setting/showing column-level " +
"privileges for table '%s'. Verify that both table and columns exist " +
- "and that you have permissions to issue a GRANT/REVOKE statement.",
+ "and that you have permissions to issue a GRANT/REVOKE/SHOW GRANT statement.",
tableName_.toString()));
}
}
@@ -284,9 +284,9 @@ public class PrivilegeSpec extends StmtNode {
} catch (TableLoadingException e) {
throw new AnalysisException(e.getMessage(), e);
} catch (AnalysisException e) {
- throw new AnalysisException(String.format("Error setting privileges for " +
+ throw new AnalysisException(String.format("Error setting/showing privileges for " +
"table '%s'. Verify that the table exists and that you have permissions " +
- "to issue a GRANT/REVOKE statement.", tableName_.toString()));
+ "to issue a GRANT/REVOKE/SHOW GRANT statement.", tableName_.toString()));
}
Preconditions.checkNotNull(table);
return table;
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 1532965..409a8d9 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
@@ -115,6 +115,20 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
"Authorization is not enabled.");
AnalysisError("SHOW GRANT ROLE myRole ON SERVER", authDisabledCtx,
"Authorization is not enabled.");
+
+ // Database, table, and column do not exist.
+ AnalysisError(String.format("SHOW GRANT %s on DATABASE foo", type),
+ "Error setting/showing privileges for database 'foo'. " +
+ "Verify that the database exists and that you have permissions to issue " +
+ "a GRANT/REVOKE/SHOW GRANT statement.");
+ AnalysisError(String.format("SHOW GRANT %s on TABLE foo.bar", type),
+ "Error setting/showing privileges for table 'foo.bar'. Verify that the " +
+ "table exists and that you have permissions to issue a " +
+ "GRANT/REVOKE/SHOW GRANT statement.");
+ AnalysisError(String.format("SHOW GRANT %s on COLUMN foo.bar.baz", type),
+ "Error setting/showing privileges for table 'foo.bar'. Verify that the " +
+ "table exists and that you have permissions to issue a " +
+ "GRANT/REVOKE/SHOW GRANT statement.");
}
// Determining if a user exists on the system is done in the AuthorizationPolicy and
@@ -173,13 +187,13 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
AnalysisError(String.format("%s ALL ON URI 'xxxx:////abc//123' %s %s",
formatArgs), "No FileSystem for scheme: xxxx");
AnalysisError(String.format("%s ALL ON DATABASE does_not_exist %s %s",
- formatArgs), "Error setting privileges for database 'does_not_exist'. " +
- "Verify that the database exists and that you have permissions to issue " +
- "a GRANT/REVOKE statement.");
+ formatArgs), "Error setting/showing privileges for " +
+ "database 'does_not_exist'. Verify that the database exists and that you " +
+ "have permissions to issue a GRANT/REVOKE/SHOW GRANT statement.");
AnalysisError(String.format("%s ALL ON TABLE does_not_exist %s %s",
- formatArgs), "Error setting privileges for table 'does_not_exist'. " +
+ formatArgs), "Error setting/showing privileges for table 'does_not_exist'. " +
"Verify that the table exists and that you have permissions to issue " +
- "a GRANT/REVOKE statement.");
+ "a GRANT/REVOKE/SHOW GRANT statement.");
AnalysisError(String.format("%s ALL ON SERVER does_not_exist %s %s",
formatArgs), "Specified server name 'does_not_exist' does not match the " +
"configured server name 'server1'");
@@ -237,14 +251,15 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
"are allowed in a column privilege spec.");
// Columns/table that don't exist
AnalysisError(String.format("%s SELECT (invalid_col) ON TABLE " +
- "functional.alltypes %s %s", formatArgs), "Error setting column-level " +
- "privileges for table 'functional.alltypes'. Verify that both table and " +
- "columns exist and that you have permissions to issue a GRANT/REVOKE " +
- "statement.");
+ "functional.alltypes %s %s", formatArgs), "Error setting/showing " +
+ "column-level privileges for table 'functional.alltypes'. Verify that " +
+ "both table and columns exist and that you have permissions to issue a " +
+ "GRANT/REVOKE/SHOW GRANT statement.");
AnalysisError(String.format("%s SELECT (id, int_col) ON TABLE " +
- "functional.does_not_exist %s %s", formatArgs), "Error setting " +
+ "functional.does_not_exist %s %s", formatArgs), "Error setting/showing " +
"privileges for table 'functional.does_not_exist'. Verify that the table " +
- "exists and that you have permissions to issue a GRANT/REVOKE statement.");
+ "exists and that you have permissions to issue a " +
+ "GRANT/REVOKE/SHOW GRANT statement.");
// REFRESH privilege
AnalyzesOk(String.format(