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/11/01 16:04:09 UTC
[4/7] impala git commit: IMPALA-2566: Remove ability to access impala
builtin cast functions directly
IMPALA-2566: Remove ability to access impala builtin cast functions
directly
Currently, all impala builtin cast functions are exposed to the user
and can be used directly in sql statements. These methods are used
internally for casting and sometimes require context not available
when used directly. This patch removes direct access to those methods
by throwing an analysis exception.
Testing:
Added relevant frontend tests.
Change-Id: I6f562607aaaf728fc417066565811bbd269baaa8
Reviewed-on: http://gerrit.cloudera.org:8080/11830
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/85526157
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/85526157
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/85526157
Branch: refs/heads/master
Commit: 85526157bc87f129c9390a46b086f597714f3717
Parents: aff174c
Author: Bikramjeet Vig <bi...@cloudera.com>
Authored: Tue Oct 30 15:03:45 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Thu Nov 1 03:02:14 2018 +0000
----------------------------------------------------------------------
.../org/apache/impala/analysis/CastExpr.java | 5 +++-
.../impala/analysis/FunctionCallExpr.java | 13 ++++++++++
.../impala/analysis/AnalyzeStmtsTest.java | 26 ++++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/85526157/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
index 932ffe1..a674f4a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
@@ -43,6 +43,9 @@ public class CastExpr extends Expr {
// True if this cast does not change the type.
private boolean noOp_ = false;
+ // Prefix for naming cast functions.
+ protected final static String CAST_FUNCTION_PREFIX = "castto";
+
/**
* C'tor for "pre-analyzed" implicit casts.
*/
@@ -96,7 +99,7 @@ public class CastExpr extends Expr {
}
private static String getFnName(Type targetType) {
- return "castTo" + targetType.getPrimitiveType().toString();
+ return CAST_FUNCTION_PREFIX + targetType.getPrimitiveType().toString();
}
public static void initBuiltins(Db db) {
http://git-wip-us.apache.org/repos/asf/impala/blob/85526157/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
index 36ef596..2716199 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
@@ -178,6 +178,14 @@ public class FunctionCallExpr extends Expr {
public boolean isMergeAggFn() { return mergeAggInputFn_ != null; }
+ /**
+ * Returns true if this is a call to an Impala builtin cast function.
+ */
+ private boolean isBuiltinCastFunction() {
+ return fnName_.isBuiltin() &&
+ fnName_.getFunction().startsWith(CastExpr.CAST_FUNCTION_PREFIX);
+ }
+
@Override
public void resetAnalysisState() {
super.resetAnalysisState();
@@ -488,6 +496,11 @@ public class FunctionCallExpr extends Expr {
throw new AnalysisException(fnName_ + "() unknown");
}
+ if (isBuiltinCastFunction()) {
+ throw new AnalysisException(toSql() +
+ " is reserved for internal use only. Use 'cast(expr AS type)' instead.");
+ }
+
if (fnName_.getFunction().equals("count") && params_.isDistinct()) {
// Treat COUNT(DISTINCT ...) special because of how we do the rewrite.
// There is no version of COUNT() that takes more than 1 argument but after
http://git-wip-us.apache.org/repos/asf/impala/blob/85526157/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index 4fbffab..d83f9dd 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -26,7 +26,9 @@ import java.lang.reflect.Field;
import java.util.List;
import java.util.Set;
+import org.apache.impala.catalog.BuiltinsDb;
import org.apache.impala.catalog.Column;
+import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.PrimitiveType;
import org.apache.impala.catalog.ScalarType;
import org.apache.impala.catalog.Table;
@@ -34,6 +36,7 @@ import org.apache.impala.catalog.Type;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.RuntimeEnv;
+import org.apache.impala.thrift.TFunctionCategory;
import org.junit.Assert;
import org.junit.Test;
@@ -3953,4 +3956,27 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
AnalysisError(": shutdown(1.234)",
"deadline expression must be an integer type but is 'DECIMAL(4,3)': 1.234");
}
+
+ @Test
+ public void TestImpalaBuiltinCastFunctions() throws ImpalaException {
+ // Other builtins work
+ AnalyzesOk("select random(1000)");
+ // Builtin cast functions throw exception.
+ String expectedErrorSuffix =
+ " is reserved for internal use only. Use 'cast(expr AS type)' instead.";
+ List<Function> fns = BuiltinsDb.getInstance().getFunctions(TFunctionCategory.SCALAR,
+ "castto");
+ for (Function fn : fns) {
+ // The analysis throws an exception on the function name before it even gets to
+ // checking the argument types, so it doesnt matter what argument we pass
+ // to it.
+ String fn_sql_str = fn.getName() + "(\"foo\")";
+ AnalysisError("select " + fn_sql_str, fn_sql_str + expectedErrorSuffix);
+ AnalysisError("select _impala_builtins." + fn_sql_str, fn_sql_str +
+ expectedErrorSuffix);
+ }
+ // Function that starts with 'castto' but does not exist should throw the
+ // right error msg.
+ AnalysisError("select casttobar(\"foo\")", "default.casttobar() unknown");
+ }
}