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/04/11 05:34:48 UTC
[4/6] impala git commit: IMPALA-6820: Remove _impala_builtins from
catalogd
IMPALA-6820: Remove _impala_builtins from catalogd
The _impala_builtins database is initialized in the constructor of
Catalog and hence is inherited by both the CatalogServiceCatalog and
ImpaladCatalog. Since _impala_builtins is not used by the catalog
server and to avoid the overhead of managing this database during normal
metadata operations (e.g. invalidate), it is moved to the ImpaladCatalog
class.
Change-Id: I166d8086db1d2920408f38dc56fe7c70a4c143a8
Reviewed-on: http://gerrit.cloudera.org:8080/9947
Reviewed-by: Tianyi Wang <tw...@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/d28b39af
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d28b39af
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d28b39af
Branch: refs/heads/master
Commit: d28b39afae5b8f1d621dcd2e1884c0353f1c058f
Parents: 2ee914d
Author: Tianyi Wang <tw...@cloudera.com>
Authored: Wed Apr 4 15:25:27 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Apr 11 03:55:07 2018 +0000
----------------------------------------------------------------------
.../main/java/org/apache/impala/analysis/CastExpr.java | 13 ++++++++-----
fe/src/main/java/org/apache/impala/analysis/Expr.java | 3 ++-
.../org/apache/impala/analysis/ExtractFromExpr.java | 3 ++-
.../org/apache/impala/analysis/FunctionCallExpr.java | 3 ++-
.../java/org/apache/impala/analysis/FunctionName.java | 8 +++++---
.../main/java/org/apache/impala/catalog/Catalog.java | 12 ------------
.../java/org/apache/impala/catalog/ImpaladCatalog.java | 8 ++++++++
.../java/org/apache/impala/catalog/ScalarFunction.java | 4 ++--
fe/src/main/jflex/sql-scanner.flex | 2 +-
.../org/apache/impala/analysis/AnalyzeExprsTest.java | 7 ++++---
10 files changed, 34 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/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 29e1736..c9a8511 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
@@ -21,6 +21,7 @@ import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Db;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Function.CompareMode;
+import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.PrimitiveType;
import org.apache.impala.catalog.ScalarFunction;
import org.apache.impala.catalog.ScalarType;
@@ -258,18 +259,20 @@ public class CastExpr extends Expr {
noOp_ = childType.equals(type_);
if (noOp_) return;
- FunctionName fnName = new FunctionName(Catalog.BUILTINS_DB, getFnName(type_));
+ FunctionName fnName = new FunctionName(ImpaladCatalog.BUILTINS_DB, getFnName(type_));
Type[] args = { childType };
Function searchDesc = new Function(fnName, args, Type.INVALID, false);
if (isImplicit_) {
- fn_ = Catalog.getBuiltin(searchDesc, CompareMode.IS_NONSTRICT_SUPERTYPE_OF);
+ fn_ = ImpaladCatalog.getBuiltinsDb().getFunction(searchDesc,
+ CompareMode.IS_NONSTRICT_SUPERTYPE_OF);
Preconditions.checkState(fn_ != null);
} else {
- fn_ = Catalog.getBuiltin(searchDesc, CompareMode.IS_IDENTICAL);
+ fn_ = ImpaladCatalog.getBuiltinsDb().getFunction(searchDesc,
+ CompareMode.IS_IDENTICAL);
if (fn_ == null) {
// allow for promotion from CHAR to STRING; only if no exact match is found
- fn_ = Catalog.getBuiltin(searchDesc.promoteCharsToStrings(),
- CompareMode.IS_IDENTICAL);
+ fn_ = ImpaladCatalog.getBuiltinsDb().getFunction(
+ searchDesc.promoteCharsToStrings(), CompareMode.IS_IDENTICAL);
}
}
if (fn_ == null) {
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/java/org/apache/impala/analysis/Expr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java
index e1fb5a8..ef53476 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java
@@ -29,6 +29,7 @@ import org.apache.impala.analysis.BinaryPredicate.Operator;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Function.CompareMode;
+import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.PrimitiveType;
import org.apache.impala.catalog.ScalarType;
import org.apache.impala.catalog.Type;
@@ -407,7 +408,7 @@ abstract public class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
*/
protected Function getBuiltinFunction(Analyzer analyzer, String name,
Type[] argTypes, CompareMode mode) throws AnalysisException {
- FunctionName fnName = new FunctionName(Catalog.BUILTINS_DB, name);
+ FunctionName fnName = new FunctionName(ImpaladCatalog.BUILTINS_DB, name);
Function searchDesc = new Function(fnName, argTypes, Type.INVALID, false);
return analyzer.getCatalog().getFunction(searchDesc, mode);
}
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java b/fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
index 5e43f9f..eea48f8 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
@@ -20,6 +20,7 @@ package org.apache.impala.analysis;
import java.util.Set;
import org.apache.impala.catalog.Catalog;
+import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.Type;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.thrift.TExtractField;
@@ -73,7 +74,7 @@ public class ExtractFromExpr extends FunctionCallExpr {
+ " does not accept the keyword FROM.");
}
if ((getFnName().getDb() != null)
- && !getFnName().getDb().equals(Catalog.BUILTINS_DB)) {
+ && !getFnName().getDb().equals(ImpaladCatalog.BUILTINS_DB)) {
throw new AnalysisException("Function " + getFnName().toString() + " conflicts " +
"with the EXTRACT builtin.");
}
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/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 12a34f7..46000b3 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
@@ -24,6 +24,7 @@ import org.apache.impala.catalog.AggregateFunction;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Db;
import org.apache.impala.catalog.Function;
+import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.ScalarFunction;
import org.apache.impala.catalog.ScalarType;
import org.apache.impala.catalog.Type;
@@ -111,7 +112,7 @@ public class FunctionCallExpr extends Expr {
return fnName.getFnNamePath().size() == 1
&& fnName.getFnNamePath().get(0).equalsIgnoreCase(name)
|| fnName.getFnNamePath().size() == 2
- && fnName.getFnNamePath().get(0).equals(Catalog.BUILTINS_DB)
+ && fnName.getFnNamePath().get(0).equals(ImpaladCatalog.BUILTINS_DB)
&& fnName.getFnNamePath().get(1).equalsIgnoreCase(name);
}
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/java/org/apache/impala/analysis/FunctionName.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/FunctionName.java b/fe/src/main/java/org/apache/impala/analysis/FunctionName.java
index 0ce8735..3ec4e63 100644
--- a/fe/src/main/java/org/apache/impala/analysis/FunctionName.java
+++ b/fe/src/main/java/org/apache/impala/analysis/FunctionName.java
@@ -21,6 +21,7 @@ import java.util.ArrayList;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Db;
+import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.thrift.TFunctionName;
import com.google.common.base.Joiner;
@@ -118,15 +119,16 @@ public class FunctionName {
}
// Resolve the database for this function.
- Db builtinDb = analyzer.getCatalog().getBuiltinsDb();
+ Db builtinDb = ImpaladCatalog.getBuiltinsDb();
if (!isFullyQualified()) {
db_ = analyzer.getDefaultDb();
if (preferBuiltinsDb && builtinDb.containsFunction(fn_)) {
- db_ = Catalog.BUILTINS_DB;
+ db_ = ImpaladCatalog.BUILTINS_DB;
}
}
Preconditions.checkNotNull(db_);
- isBuiltin_ = db_.equals(Catalog.BUILTINS_DB) && builtinDb.containsFunction(fn_);
+ isBuiltin_ = db_.equals(ImpaladCatalog.BUILTINS_DB) &&
+ builtinDb.containsFunction(fn_);
isAnalyzed_ = true;
}
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/java/org/apache/impala/catalog/Catalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/Catalog.java b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
index 6f9cdb5..aba88df 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Catalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
@@ -59,7 +59,6 @@ public abstract class Catalog {
// Initial catalog version.
public final static long INITIAL_CATALOG_VERSION = 0L;
public static final String DEFAULT_DB = "default";
- public static final String BUILTINS_DB = "_impala_builtins";
protected final MetaStoreClientPool metaStoreClientPool_ =
new MetaStoreClientPool(0, 0);
@@ -75,9 +74,6 @@ public abstract class Catalog {
new AtomicReference<ConcurrentHashMap<String, Db>>(
new ConcurrentHashMap<String, Db>());
- // DB that contains all builtins
- private static Db builtinsDb_;
-
// Cache of data sources.
protected final CatalogObjectCache<DataSource> dataSources_;
@@ -88,8 +84,6 @@ public abstract class Catalog {
public Catalog() {
dataSources_ = new CatalogObjectCache<DataSource>();
- builtinsDb_ = new BuiltinsDb(BUILTINS_DB);
- addDb(builtinsDb_);
}
/**
@@ -104,8 +98,6 @@ public abstract class Catalog {
metaStoreClientPool_.initClients(numClients, initialCnxnTimeoutSec);
}
- public Db getBuiltinsDb() { return builtinsDb_; }
-
/**
* Adds a new database to the catalog, replacing any existing database with the same
* name. Returns the previous database with this name, or null if there was no
@@ -288,10 +280,6 @@ public abstract class Catalog {
return db.getFunction(desc, mode);
}
- public static Function getBuiltin(Function desc, Function.CompareMode mode) {
- return builtinsDb_.getFunction(desc, mode);
- }
-
/**
* Removes a function from the catalog. Increments the catalog version and returns
* the Function object that was removed if the function existed, otherwise returns
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
index fce4574..6ad4234 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
@@ -76,6 +76,7 @@ import com.google.common.base.Preconditions;
public class ImpaladCatalog extends Catalog {
private static final Logger LOG = Logger.getLogger(ImpaladCatalog.class);
private static final TUniqueId INITIAL_CATALOG_SERVICE_ID = new TUniqueId(0L, 0L);
+ public static final String BUILTINS_DB = "_impala_builtins";
// The last known Catalog Service ID. If the ID changes, it indicates the CatalogServer
// has restarted.
@@ -98,8 +99,13 @@ public class ImpaladCatalog extends Catalog {
// Used during table creation.
private final String defaultKuduMasterHosts_;
+ // DB that contains all builtins
+ private static Db builtinsDb_;
+
public ImpaladCatalog(String defaultKuduMasterHosts) {
super();
+ builtinsDb_ = new BuiltinsDb(BUILTINS_DB);
+ addDb(builtinsDb_);
defaultKuduMasterHosts_ = defaultKuduMasterHosts;
// Ensure the contents of the CatalogObjectVersionQueue instance are cleared when a
// new instance of ImpaladCatalog is created (see IMPALA-6486).
@@ -536,4 +542,6 @@ public class ImpaladCatalog extends Catalog {
}
}
public TUniqueId getCatalogServiceId() { return catalogServiceId_; }
+
+ public static Db getBuiltinsDb() { return builtinsDb_; }
}
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java b/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
index 8872e1e..5035d38 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
@@ -72,7 +72,7 @@ public class ScalarFunction extends Function {
String prepareFnSymbol, String closeFnSymbol, boolean userVisible) {
Preconditions.checkNotNull(symbol);
ScalarFunction fn = new ScalarFunction(
- new FunctionName(Catalog.BUILTINS_DB, name), argTypes, retType, hasVarArgs);
+ new FunctionName(ImpaladCatalog.BUILTINS_DB, name), argTypes, retType, hasVarArgs);
fn.setBinaryType(TFunctionBinaryType.BUILTIN);
fn.setUserVisible(userVisible);
fn.setIsPersistent(true);
@@ -221,7 +221,7 @@ public class ScalarFunction extends Function {
ArrayList<Type> argTypes, boolean hasVarArgs, Type retType,
boolean userVisible) {
ScalarFunction fn = new ScalarFunction(
- new FunctionName(Catalog.BUILTINS_DB, name), argTypes, retType, hasVarArgs);
+ new FunctionName(ImpaladCatalog.BUILTINS_DB, name), argTypes, retType, hasVarArgs);
fn.setBinaryType(TFunctionBinaryType.BUILTIN);
fn.setUserVisible(userVisible);
fn.setIsPersistent(true);
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/main/jflex/sql-scanner.flex
----------------------------------------------------------------------
diff --git a/fe/src/main/jflex/sql-scanner.flex b/fe/src/main/jflex/sql-scanner.flex
index c4ed217..dd1da7c 100644
--- a/fe/src/main/jflex/sql-scanner.flex
+++ b/fe/src/main/jflex/sql-scanner.flex
@@ -30,7 +30,7 @@ import java.util.HashSet;
import org.apache.impala.analysis.SqlParserSymbols;
import org.apache.impala.catalog.BuiltinsDb;
-import static org.apache.impala.catalog.Catalog.BUILTINS_DB;
+import static org.apache.impala.catalog.ImpaladCatalog.BUILTINS_DB;
import org.apache.impala.service.BackendConfig;
import org.apache.impala.thrift.TReservedWordsVersion;
http://git-wip-us.apache.org/repos/asf/impala/blob/d28b39af/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
index cc71438..df99a62 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
@@ -34,6 +34,7 @@ import org.apache.impala.catalog.Column;
import org.apache.impala.catalog.Db;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Function.CompareMode;
+import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.PrimitiveType;
import org.apache.impala.catalog.ScalarFunction;
import org.apache.impala.catalog.ScalarType;
@@ -2690,7 +2691,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
@Test
// IMPALA-2233: Regression test for loss of precision through implicit casts.
public void TestImplicitArgumentCasts() throws AnalysisException {
- FunctionName fnName = new FunctionName(Catalog.BUILTINS_DB, "greatest");
+ FunctionName fnName = new FunctionName(ImpaladCatalog.BUILTINS_DB, "greatest");
Function tinyIntFn = new Function(fnName, new Type[] {ScalarType.DOUBLE},
Type.DOUBLE, true);
Function decimalFn = new Function(fnName,
@@ -2700,7 +2701,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
Assert.assertTrue(tinyIntFn.compare(decimalFn,
CompareMode.IS_NONSTRICT_SUPERTYPE_OF));
// Check that this resolves to the decimal version of the function.
- Db db = catalog_.getDb(Catalog.BUILTINS_DB);
+ Db db = catalog_.getDb(ImpaladCatalog.BUILTINS_DB);
Function foundFn = db.getFunction(decimalFn, CompareMode.IS_NONSTRICT_SUPERTYPE_OF);
assertNotNull(foundFn);
Assert.assertTrue(foundFn.getArgs()[0].isDecimal());
@@ -2733,7 +2734,7 @@ public class AnalyzeExprsTest extends AnalyzerTest {
Assert.assertNotNull(foundFn);
Assert.assertEquals(Type.DOUBLE, foundFn.getArgs()[0]);
- FunctionName lagFnName = new FunctionName(Catalog.BUILTINS_DB, "lag");
+ FunctionName lagFnName = new FunctionName(ImpaladCatalog.BUILTINS_DB, "lag");
// Timestamp should not be converted to string if string overload available.
Function lagStringFn = new Function(lagFnName,
new Type[] {ScalarType.STRING, Type.TINYINT}, Type.INVALID, false);