This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 85906dd6ac141c1015d1abebef1228c9b8b66d90
Author: Todd Lipcon <t...@cloudera.com>
AuthorDate: Wed Jun 6 10:30:34 2018 -0700
IMPALA-7128 (part 2): add an interface for data sources
This changes most of the usage of DataSource and DataSourceTable to use
interfaces instead of implementation classes. There are still various
usages of the implementation for functionality like creating and
dropping data sources. We'll address those as part of IMPALA-7131 at a
later date.
Change-Id: Ibe704197dc2ad7c09b8340865f17567096aa630e
Reviewed-on: http://gerrit.cloudera.org:8080/10626
Reviewed-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
---
.../org/apache/impala/analysis/AlterTableStmt.java | 3 +-
.../java/org/apache/impala/analysis/Analyzer.java | 3 +-
.../impala/analysis/CreateTableDataSrcStmt.java | 4 +--
.../org/apache/impala/analysis/PrivilegeSpec.java | 4 +--
.../java/org/apache/impala/catalog/DataSource.java | 8 ++---
.../org/apache/impala/catalog/DataSourceTable.java | 18 ++++++----
.../java/org/apache/impala/catalog/FeCatalog.java | 5 ++-
.../org/apache/impala/catalog/FeDataSource.java | 31 +++++++++++++++++
.../apache/impala/catalog/FeDataSourceTable.java | 39 ++++++++++++++++++++++
.../apache/impala/planner/DataSourceScanNode.java | 6 ++--
.../apache/impala/planner/SingleNodePlanner.java | 4 +--
.../apache/impala/service/CatalogOpExecutor.java | 2 ++
.../java/org/apache/impala/service/Frontend.java | 10 +++---
.../org/apache/impala/service/JniFrontend.java | 5 +--
14 files changed, 110 insertions(+), 32 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
b/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
index eda260f..a173975 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
@@ -21,6 +21,7 @@ import java.util.List;
import org.apache.impala.authorization.Privilege;
import org.apache.impala.catalog.DataSourceTable;
+import org.apache.impala.catalog.FeDataSourceTable;
import org.apache.impala.catalog.FeTable;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.thrift.TAlterTableParams;
@@ -90,7 +91,7 @@ public abstract class AlterTableStmt extends StatementBase {
}
Preconditions.checkState(tableRef instanceof BaseTableRef);
table_ = tableRef.getTable();
- if (table_ instanceof DataSourceTable
+ if (table_ instanceof FeDataSourceTable
&& !(this instanceof AlterTableSetColumnStats)) {
throw new AnalysisException(String.format(
"ALTER TABLE not allowed on a table produced by a data source: %s",
diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index f0f7334..67b9f59 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -40,6 +40,7 @@ import org.apache.impala.catalog.Column;
import org.apache.impala.catalog.DataSourceTable;
import org.apache.impala.catalog.DatabaseNotFoundException;
import org.apache.impala.catalog.FeCatalog;
+import org.apache.impala.catalog.FeDataSourceTable;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.FeFsTable;
import org.apache.impala.catalog.FeTable;
@@ -589,7 +590,7 @@ public class Analyzer {
Preconditions.checkState(table instanceof FeFsTable ||
table instanceof KuduTable ||
table instanceof HBaseTable ||
- table instanceof DataSourceTable);
+ table instanceof FeDataSourceTable);
return new BaseTableRef(tableRef, resolvedPath);
} else {
return new CollectionTableRef(tableRef, resolvedPath);
diff --git
a/fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
b/fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
index 1df8280..00fd4b6 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableDataSrcStmt.java
@@ -24,8 +24,8 @@ import static
org.apache.impala.catalog.DataSourceTable.TBL_PROP_INIT_STRING;
import static org.apache.impala.catalog.DataSourceTable.TBL_PROP_LOCATION;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.catalog.DataSource;
import org.apache.impala.catalog.DataSourceTable;
+import org.apache.impala.catalog.FeDataSource;
import org.apache.impala.common.AnalysisException;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
@@ -51,7 +51,7 @@ public class CreateTableDataSrcStmt extends CreateTableStmt {
public void analyze(Analyzer analyzer) throws AnalysisException {
super.analyze(analyzer);
String dataSourceName = getTblProperties().get(TBL_PROP_DATA_SRC_NAME);
- DataSource dataSource =
analyzer.getCatalog().getDataSource(dataSourceName);
+ FeDataSource dataSource =
analyzer.getCatalog().getDataSource(dataSourceName);
if (dataSource == null) {
throw new AnalysisException("Data source does not exist: " +
dataSourceName);
}
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 610ffa9..fb75398 100644
--- a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
+++ b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
@@ -20,7 +20,7 @@ package org.apache.impala.analysis;
import java.util.List;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.catalog.DataSourceTable;
+import org.apache.impala.catalog.FeDataSourceTable;
import org.apache.impala.catalog.FeTable;
import org.apache.impala.catalog.FeView;
import org.apache.impala.catalog.RolePrivilege;
@@ -242,7 +242,7 @@ public class PrivilegeSpec implements ParseNode {
throw new AnalysisException("Column-level privileges on views are not " +
"supported.");
}
- if (table instanceof DataSourceTable) {
+ if (table instanceof FeDataSourceTable) {
throw new AnalysisException("Column-level privileges on external data " +
"source tables are not supported.");
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/DataSource.java
b/fe/src/main/java/org/apache/impala/catalog/DataSource.java
index f59f3be..527b187 100644
--- a/fe/src/main/java/org/apache/impala/catalog/DataSource.java
+++ b/fe/src/main/java/org/apache/impala/catalog/DataSource.java
@@ -17,8 +17,6 @@
package org.apache.impala.catalog;
-import org.apache.hadoop.fs.Path;
-
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.thrift.TDataSource;
@@ -28,7 +26,7 @@ import com.google.common.base.Objects;
* Represents a data source in the catalog. Contains the data source name and
all
* information needed to locate and load the data source.
*/
-public class DataSource extends CatalogObjectImpl {
+public class DataSource extends CatalogObjectImpl implements FeDataSource {
private final String dataSrcName_;
private final String className_;
private final String apiVersionString_;
@@ -57,9 +55,11 @@ public class DataSource extends CatalogObjectImpl {
public String getName() { return dataSrcName_; }
@Override
public String getUniqueName() { return "DATA_SOURCE:" +
dataSrcName_.toLowerCase(); }
-
+ @Override // FeDataSource
public String getLocation() { return location_; }
+ @Override // FeDataSource
public String getClassName() { return className_; }
+ @Override // FeDataSource
public String getApiVersion() { return apiVersionString_; }
public TDataSource toThrift() {
diff --git a/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
b/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
index e9e26b6..3ed5b54 100644
--- a/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
@@ -39,14 +39,14 @@ import org.apache.impala.util.TResultRowBuilder;
import com.google.common.base.Preconditions;
/**
- * Represents a table backed by an external data source. All data source
properties are
- * stored as table properties (persisted in the metastore) because the
DataSource catalog
- * object is not persisted so the DataSource catalog object will not exist if
the catalog
- * server is restarted, but the table does not need the DataSource catalog
object in
- * order to scan the table. Tables that contain the TBL_PROP_DATA_SRC_NAME
table
- * parameter are assumed to be backed by an external data source.
+ * All data source properties are stored as table properties (persisted in the
+ * metastore) because the DataSource catalog object is not persisted so the
+ * DataSource catalog object will not exist if the catalog server is restarted,
+ * but the table does not need the DataSource catalog object in order to scan
+ * the table. Tables that contain the TBL_PROP_DATA_SRC_NAME table parameter
are
+ * assumed to be backed by an external data source.
*/
-public class DataSourceTable extends Table {
+public class DataSourceTable extends Table implements FeDataSourceTable {
private final static Logger LOG =
LoggerFactory.getLogger(DataSourceTable.class);
/**
@@ -85,13 +85,16 @@ public class DataSourceTable extends Table {
/**
* Gets the the data source.
*/
+ @Override // FeDataSourceTable
public TDataSource getDataSource() { return dataSource_; }
/**
* Gets the table init string passed to the data source.
*/
+ @Override // FeDataSourceTable
public String getInitString() { return initString_; }
+ @Override // FeDataSourceTable
public int getNumNodes() { return 1; }
@Override
@@ -210,6 +213,7 @@ public class DataSourceTable extends Table {
* SHOW TABLE STATS statement. The schema of the returned TResultSet is set
* inside this method.
*/
+ @Override // FeDataSourceTable
public TResultSet getTableStats() {
TResultSet result = new TResultSet();
TResultSetMetadata resultSchema = new TResultSetMetadata();
diff --git a/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
b/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
index 57aa6f2..c98f6fc 100644
--- a/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
@@ -56,11 +56,10 @@ public interface FeCatalog {
List<TPartitionKeyValue> partition_spec) throws CatalogException;
/** @see Catalog#getDataSources(PatternMatcher) */
- List<DataSource> getDataSources(PatternMatcher createHivePatternMatcher);
+ List<? extends FeDataSource> getDataSources(PatternMatcher
createHivePatternMatcher);
/** @see Catalog#getDataSource(String) */
- // TODO(todd): introduce FeDataSource
- public DataSource getDataSource(String dataSourceName);
+ public FeDataSource getDataSource(String dataSourceName);
/** @see Catalog#getFunction(Function, Function.CompareMode) */
// TODO(todd): introduce FeFunction
diff --git a/fe/src/main/java/org/apache/impala/catalog/FeDataSource.java
b/fe/src/main/java/org/apache/impala/catalog/FeDataSource.java
new file mode 100644
index 0000000..6c30cca
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/FeDataSource.java
@@ -0,0 +1,31 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.impala.catalog;
+
+/**
+ * Frontend interface for interacting with data sources.
+ */
+public interface FeDataSource {
+ String getName();
+
+ String getLocation();
+
+ String getClassName();
+
+ String getApiVersion();
+}
diff --git a/fe/src/main/java/org/apache/impala/catalog/FeDataSourceTable.java
b/fe/src/main/java/org/apache/impala/catalog/FeDataSourceTable.java
new file mode 100644
index 0000000..a76f38d
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/FeDataSourceTable.java
@@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.impala.catalog;
+
+import org.apache.impala.thrift.TDataSource;
+import org.apache.impala.thrift.TResultSet;
+
+/**
+ * Represents a table backed by an external data source.
+ */
+public interface FeDataSourceTable extends FeTable {
+
+ TDataSource getDataSource();
+
+ String getInitString();
+
+ int getNumNodes();
+
+ // TODO(todd): it seems like all FeTables implement this, perhaps
+ // this should just be a method on FeTable and simplify the code
+ // in Frontend.getTableStats?
+ TResultSet getTableStats();
+
+}
diff --git a/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
b/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
index 9175fbb..a41630b 100644
--- a/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
@@ -33,7 +33,7 @@ import org.apache.impala.analysis.SlotRef;
import org.apache.impala.analysis.StringLiteral;
import org.apache.impala.analysis.TupleDescriptor;
import org.apache.impala.catalog.DataSource;
-import org.apache.impala.catalog.DataSourceTable;
+import org.apache.impala.catalog.FeDataSourceTable;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.InternalException;
import org.apache.impala.extdatasource.ExternalDataSourceExecutor;
@@ -69,7 +69,7 @@ import com.google.common.collect.Lists;
public class DataSourceScanNode extends ScanNode {
private final static Logger LOG =
LoggerFactory.getLogger(DataSourceScanNode.class);
private final TupleDescriptor desc_;
- private final DataSourceTable table_;
+ private final FeDataSourceTable table_;
// The converted conjuncts_ that were accepted by the data source. A
conjunct can
// be converted if it contains only disjunctive predicates of the form
@@ -87,7 +87,7 @@ public class DataSourceScanNode extends ScanNode {
public DataSourceScanNode(PlanNodeId id, TupleDescriptor desc, List<Expr>
conjuncts) {
super(id, desc, "SCAN DATA SOURCE");
desc_ = desc;
- table_ = (DataSourceTable) desc_.getTable();
+ table_ = (FeDataSourceTable) desc_.getTable();
conjuncts_ = conjuncts;
acceptedPredicates_ = null;
acceptedConjuncts_ = null;
diff --git a/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
b/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
index 3a1c956..331c910 100644
--- a/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
+++ b/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
@@ -53,7 +53,7 @@ import org.apache.impala.analysis.TupleIsNullPredicate;
import org.apache.impala.analysis.UnionStmt;
import org.apache.impala.analysis.UnionStmt.UnionOperand;
import org.apache.impala.catalog.ColumnStats;
-import org.apache.impala.catalog.DataSourceTable;
+import org.apache.impala.catalog.FeDataSourceTable;
import org.apache.impala.catalog.FeFsPartition;
import org.apache.impala.catalog.FeFsTable;
import org.apache.impala.catalog.FeTable;
@@ -1305,7 +1305,7 @@ public class SingleNodePlanner {
FeTable table = tblRef.getTable();
if (table instanceof FeFsTable) {
return createHdfsScanPlan(tblRef, aggInfo, conjuncts, analyzer);
- } else if (table instanceof DataSourceTable) {
+ } else if (table instanceof FeDataSourceTable) {
scanNode = new DataSourceScanNode(ctx_.getNextNodeId(), tblRef.getDesc(),
conjuncts);
scanNode.init(analyzer);
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 80ebd19..2d83bab 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1113,6 +1113,7 @@ public class CatalogOpExecutor {
private void createDataSource(TCreateDataSourceParams params,
TDdlExecResponse resp)
throws ImpalaException {
+ // TODO(IMPALA-7131): support data sources with LocalCatalog.
if (LOG.isTraceEnabled()) { LOG.trace("Adding DATA SOURCE: " +
params.toString()); }
DataSource dataSource = DataSource.fromThrift(params.getData_source());
DataSource existingDataSource =
catalog_.getDataSource(dataSource.getName());
@@ -1134,6 +1135,7 @@ public class CatalogOpExecutor {
private void dropDataSource(TDropDataSourceParams params, TDdlExecResponse
resp)
throws ImpalaException {
+ // TODO(IMPALA-7131): support data sources with LocalCatalog.
if (LOG.isTraceEnabled()) LOG.trace("Drop DATA SOURCE: " +
params.toString());
DataSource dataSource = catalog_.removeDataSource(params.getData_source());
if (dataSource == null) {
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java
b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 54a9473..ad51286 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -75,10 +75,10 @@ import org.apache.impala.authorization.User;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.Column;
-import org.apache.impala.catalog.DataSource;
-import org.apache.impala.catalog.DataSourceTable;
import org.apache.impala.catalog.DatabaseNotFoundException;
import org.apache.impala.catalog.Db;
+import org.apache.impala.catalog.FeDataSource;
+import org.apache.impala.catalog.FeDataSourceTable;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.FeFsTable;
import org.apache.impala.catalog.Function;
@@ -691,7 +691,7 @@ public class Frontend {
* Returns all data sources that match the pattern. If pattern is null,
* matches all data sources.
*/
- public List<DataSource> getDataSrcs(String pattern) {
+ public List<? extends FeDataSource> getDataSrcs(String pattern) {
return impaladCatalog_.get().getDataSources(
PatternMatcher.createHivePatternMatcher(pattern));
}
@@ -734,8 +734,8 @@ public class Frontend {
return ((FeFsTable) table).getTableStats();
} else if (table instanceof HBaseTable) {
return ((HBaseTable) table).getTableStats();
- } else if (table instanceof DataSourceTable) {
- return ((DataSourceTable) table).getTableStats();
+ } else if (table instanceof FeDataSourceTable) {
+ return ((FeDataSourceTable) table).getTableStats();
} else if (table instanceof KuduTable) {
if (op == TShowStatsOp.RANGE_PARTITIONS) {
return ((KuduTable) table).getRangePartitions();
diff --git a/fe/src/main/java/org/apache/impala/service/JniFrontend.java
b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
index a343182..d8ef912 100644
--- a/fe/src/main/java/org/apache/impala/service/JniFrontend.java
+++ b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
@@ -46,6 +46,7 @@ import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.authorization.ImpalaInternalAdminUser;
import org.apache.impala.authorization.User;
import org.apache.impala.catalog.DataSource;
+import org.apache.impala.catalog.FeDataSource;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Role;
@@ -319,12 +320,12 @@ public class JniFrontend {
JniUtil.deserializeThrift(protocolFactory_, params, thriftParams);
TGetDataSrcsResult result = new TGetDataSrcsResult();
- List<DataSource> dataSources = frontend_.getDataSrcs(params.pattern);
+ List<? extends FeDataSource> dataSources =
frontend_.getDataSrcs(params.pattern);
result.setData_src_names(Lists.<String>newArrayListWithCapacity(dataSources.size()));
result.setLocations(Lists.<String>newArrayListWithCapacity(dataSources.size()));
result.setClass_names(Lists.<String>newArrayListWithCapacity(dataSources.size()));
result.setApi_versions(Lists.<String>newArrayListWithCapacity(dataSources.size()));
- for (DataSource dataSource: dataSources) {
+ for (FeDataSource dataSource: dataSources) {
result.addToData_src_names(dataSource.getName());
result.addToLocations(dataSource.getLocation());
result.addToClass_names(dataSource.getClassName());
