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/05/14 22:43:00 UTC
[impala] branch 2.x updated (65189dd -> aea18dd)
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a change to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git.
from 65189dd IMPALA-7140 (part 7): small fixes to enable most queries on HDFS tables
new cb47554 IMPALA-7140 (part 8): support views in LocalCatalog
new 3afde5d IMPALA-7201. Support DDL with LocalCatalog enabled
new 2e720ac IMPALA-6086: Use of permanent function should require SELECT privilege on DB
new 43e5450 IMPALA-7228: Add tpcds-unmodified to single-node-perf-run
new d0a6239 IMPALA-6810: runtime_row_filters.test: omit pool name in pattern
new aea18dd IMPALA-7288: Fix Codegen Crash in FinalizeModule() (Addendum)
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/codegen/llvm-codegen.cc | 5 +-
be/src/codegen/llvm-codegen.h | 4 +-
bin/single_node_perf_run.py | 3 +-
.../apache/impala/analysis/AnalysisContext.java | 13 ++++
.../main/java/org/apache/impala/catalog/View.java | 54 ++++++--------
.../org/apache/impala/catalog/local/LocalDb.java | 19 +++--
.../apache/impala/catalog/local/LocalTable.java | 3 +-
.../org/apache/impala/catalog/local/LocalView.java | 83 ++++++++++++++++++++++
.../apache/impala/service/CatalogOpExecutor.java | 22 +++++-
.../apache/impala/service/FeCatalogManager.java | 7 +-
.../impala/analysis/AuthorizationStmtTest.java | 41 +++++++++--
.../impala/catalog/local/LocalCatalogTest.java | 9 +++
.../queries/QueryTest/runtime_row_filters.test | 7 +-
13 files changed, 212 insertions(+), 58 deletions(-)
create mode 100644 fe/src/main/java/org/apache/impala/catalog/local/LocalView.java
[impala] 02/06: IMPALA-7201. Support DDL with LocalCatalog enabled
Posted by ta...@apache.org.
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 3afde5d99e7bc434358b813457d83cac4a6f086c
Author: Todd Lipcon <to...@cloudera.com>
AuthorDate: Fri Jun 22 16:33:44 2018 -0700
IMPALA-7201. Support DDL with LocalCatalog enabled
This fixes a couple issues with DDL commands when LocalCatalog is
enabled:
- updateCatalogCache() gets called after any DDL. Instead of throwing an
exception, we can just no-op this by returning some fake result.
- In order to support 'drop database' we need to properly implement the
various function-related calls such that they don't throw exceptions.
This changes them to be stubbed out as having no functions.
- Fixes for 'alter view' and 'drop view' so that the underlying target
table gets loaded by the catalogd before attempting the operation.
Without this, in the LocalCatalog case, the catalogd would only have
an IncompleteTable and these operations would fail with "unexpected
table type" errors.
With this patch I was able to run 'run-tests.py -k views' and 3/4
passed. The one that failed depends on HBase tables, not yet
implemented.
Change-Id: Ic39c97a5f5ad145e03b96d1a470dc2dfa6ec71a5
Reviewed-on: http://gerrit.cloudera.org:8080/10806
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
---
.../org/apache/impala/catalog/local/LocalDb.java | 19 +++++++++++++------
.../apache/impala/service/CatalogOpExecutor.java | 22 ++++++++++++++++++++--
.../apache/impala/service/FeCatalogManager.java | 7 +++++--
3 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
index 2d223a6..6a779d7 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
@@ -17,6 +17,7 @@
package org.apache.impala.catalog.local;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -136,34 +137,40 @@ class LocalDb implements FeDb {
@Override
public Function getFunction(Function desc, CompareMode mode) {
- throw new UnsupportedOperationException("TODO");
+ // TODO(todd): implement functions
+ return null;
}
@Override
public List<Function> getFunctions(String functionName) {
- throw new UnsupportedOperationException("TODO");
+ // TODO(todd): implement functions
+ return Collections.emptyList();
}
@Override
public List<Function> getFunctions(
TFunctionCategory category, String function) {
- throw new UnsupportedOperationException("TODO");
+ // TODO(todd): implement functions
+ return Collections.emptyList();
}
@Override
public List<Function> getFunctions(
TFunctionCategory category, PatternMatcher patternMatcher) {
- throw new UnsupportedOperationException("TODO");
+ // TODO(todd): implement functions
+ return Collections.emptyList();
}
@Override
public int numFunctions() {
- throw new UnsupportedOperationException("TODO");
+ // TODO(todd): implement functions
+ return 0;
}
@Override
public boolean containsFunction(String function) {
- throw new UnsupportedOperationException("TODO");
+ // TODO(todd): implement functions
+ return false;
}
@Override
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 88668d7..39cc108 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -696,8 +696,9 @@ public class CatalogOpExecutor {
Preconditions.checkState(params.getColumns() != null &&
params.getColumns().size() > 0,
"Null or empty column list given as argument to DdlExecutor.alterView");
- Table tbl = catalog_.getTable(tableName.getDb(), tableName.getTbl());
- Preconditions.checkState(tbl instanceof View);
+ Table tbl = getExistingTable(tableName.getDb(), tableName.getTbl());
+ Preconditions.checkState(tbl instanceof View, "Expected view: %s",
+ tableName);
tryLock(tbl);
try {
long newCatalogVersion = catalog_.incrementAndGetCatalogVersion();
@@ -1390,6 +1391,23 @@ public class CatalogOpExecutor {
Preconditions.checkState(tableName != null && tableName.isFullyQualified());
LOG.trace(String.format("Dropping table/view %s", tableName));
+ // If the table exists, ensure that it is loaded before we try to operate on it.
+ // We do this up here rather than down below to avoid doing too much table-loading
+ // work while holding the DDL lock. We can't simply use 'getExistingTable' because
+ // we rely on more granular checks to provide the correct summary message for
+ // the 'IF EXISTS' case.
+ //
+ // In the standard catalogd implementation, the table will most likely already
+ // be loaded because the planning phase on the impalad side triggered the loading.
+ // In the LocalCatalog configuration, however, this is often necessary.
+ try {
+ catalog_.getOrLoadTable(params.getTable_name().db_name,
+ params.getTable_name().table_name);
+ } catch (CatalogException e) {
+ // Ignore exceptions -- the above was just to trigger loading. Failure to load
+ // or non-existence of the database will be handled down below.
+ }
+
TCatalogObject removedObject = new TCatalogObject();
synchronized (metastoreDdlLock_) {
Db db = catalog_.getDb(params.getTable_name().db_name);
diff --git a/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java b/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
index 2bd44a4..ef8340d 100644
--- a/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
+++ b/fe/src/main/java/org/apache/impala/service/FeCatalogManager.java
@@ -22,6 +22,7 @@ import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.FeCatalog;
import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.catalog.local.LocalCatalog;
+import org.apache.impala.thrift.TUniqueId;
import org.apache.impala.thrift.TUpdateCatalogCacheRequest;
import org.apache.impala.thrift.TUpdateCatalogCacheResponse;
import org.apache.thrift.TException;
@@ -127,8 +128,10 @@ public abstract class FeCatalogManager {
@Override
TUpdateCatalogCacheResponse updateCatalogCache(TUpdateCatalogCacheRequest req) {
- throw new IllegalStateException(
- "Unexpected call to updateCatalogCache() with local catalog enabled");
+ // TODO(todd) upon implementing caching, probably want to invalidate appropriate
+ // pieces of cached info. For now, this fake response seems enough to make
+ // the backend call site happy.
+ return new TUpdateCatalogCacheResponse(new TUniqueId(), -1, -1);
}
}
[impala] 01/06: IMPALA-7140 (part 8): support views in LocalCatalog
Posted by ta...@apache.org.
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 cb4755421b3437808037feca5c29d95f446aab93
Author: Todd Lipcon <to...@cloudera.com>
AuthorDate: Fri Jun 22 10:53:23 2018 -0700
IMPALA-7140 (part 8): support views in LocalCatalog
This adds basic support for loading views in LocalCatalog. Tested with a
small unit test and also verified from the shell that I can select from
a view.
Change-Id: Ib3516b9ceff6dce12ded68d93afde09728627e08
Reviewed-on: http://gerrit.cloudera.org:8080/10805
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
.../main/java/org/apache/impala/catalog/View.java | 54 ++++++--------
.../apache/impala/catalog/local/LocalTable.java | 3 +-
.../org/apache/impala/catalog/local/LocalView.java | 83 ++++++++++++++++++++++
.../impala/catalog/local/LocalCatalogTest.java | 9 +++
4 files changed, 116 insertions(+), 33 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/View.java b/fe/src/main/java/org/apache/impala/catalog/View.java
index 66d942a..4026d6a 100644
--- a/fe/src/main/java/org/apache/impala/catalog/View.java
+++ b/fe/src/main/java/org/apache/impala/catalog/View.java
@@ -45,24 +45,6 @@ import com.google.common.collect.Lists;
*/
public class View extends Table implements FeView {
- // The original SQL-string given as view definition. Set during analysis.
- // Corresponds to Hive's viewOriginalText.
- private String originalViewDef_;
-
- // Query statement (as SQL string) that defines the View for view substitution.
- // It is a transformation of the original view definition, e.g., to enforce the
- // explicit column definitions even if the original view definition has explicit
- // column aliases.
- // If column definitions were given, then this "expanded" view definition
- // wraps the original view definition in a select stmt as follows.
- //
- // SELECT viewName.origCol1 AS colDesc1, viewName.origCol2 AS colDesc2, ...
- // FROM (originalViewDef) AS viewName
- //
- // Corresponds to Hive's viewExpandedText, but is not identical to the SQL
- // Hive would produce in view creation.
- private String inlineViewDef_;
-
// View definition created by parsing inlineViewDef_ into a QueryStmt.
private QueryStmt queryStmt_;
@@ -117,7 +99,7 @@ public class View extends Table implements FeView {
numClusteringCols_ = 0;
tableStats_ = new TTableStats(-1);
tableStats_.setTotal_file_bytes(-1);
- init();
+ queryStmt_ = parseViewDef(this);
} catch (TableLoadingException e) {
throw e;
} catch (Exception e) {
@@ -128,22 +110,32 @@ public class View extends Table implements FeView {
@Override
protected void loadFromThrift(TTable t) throws TableLoadingException {
super.loadFromThrift(t);
- init();
+ queryStmt_ = parseViewDef(this);
}
/**
- * Initializes the originalViewDef_, inlineViewDef_, and queryStmt_ members
- * by parsing the expanded view definition SQL-string.
+ * Parse the expanded view definition SQL-string.
* Throws a TableLoadingException if there was any error parsing the
* the SQL or if the view definition did not parse into a QueryStmt.
*/
- private void init() throws TableLoadingException {
- // Set view-definition SQL strings.
- originalViewDef_ = getMetaStoreTable().getViewOriginalText();
- inlineViewDef_ = getMetaStoreTable().getViewExpandedText();
+ public static QueryStmt parseViewDef(FeView view) throws TableLoadingException {
+ // Query statement (as SQL string) that defines the View for view substitution.
+ // It is a transformation of the original view definition, e.g., to enforce the
+ // explicit column definitions even if the original view definition has explicit
+ // column aliases.
+ // If column definitions were given, then this "expanded" view definition
+ // wraps the original view definition in a select stmt as follows.
+ //
+ // SELECT viewName.origCol1 AS colDesc1, viewName.origCol2 AS colDesc2, ...
+ // FROM (originalViewDef) AS viewName
+ //
+ // Corresponds to Hive's viewExpandedText, but is not identical to the SQL
+ // Hive would produce in view creation.
+ String inlineViewDef = view.getMetaStoreTable().getViewExpandedText();
+
// Parse the expanded view definition SQL-string into a QueryStmt and
// populate a view definition.
- SqlScanner input = new SqlScanner(new StringReader(inlineViewDef_));
+ SqlScanner input = new SqlScanner(new StringReader(inlineViewDef));
SqlParser parser = new SqlParser(input);
ParseNode node = null;
try {
@@ -153,14 +145,14 @@ public class View extends Table implements FeView {
// of tables that the user triggering this load may not have privileges on.
throw new TableLoadingException(
String.format("Failed to parse view-definition statement of view: " +
- "%s.%s", db_.getName(), name_));
+ "%s", view.getFullName()));
}
// Make sure the view definition parses to a query statement.
if (!(node instanceof QueryStmt)) {
- throw new TableLoadingException(String.format("View definition of %s.%s " +
- "is not a query statement", db_.getName(), name_));
+ throw new TableLoadingException(String.format("View definition of %s " +
+ "is not a query statement", view.getFullName()));
}
- queryStmt_ = (QueryStmt) node;
+ return (QueryStmt) node;
}
@Override
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
index 4b57cb8..d128ca2 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
@@ -40,7 +40,6 @@ import org.apache.impala.catalog.KuduTable;
import org.apache.impala.catalog.StructField;
import org.apache.impala.catalog.StructType;
import org.apache.impala.catalog.TableLoadingException;
-import org.apache.impala.catalog.View;
import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.thrift.TTableStats;
import org.apache.log4j.Logger;
@@ -73,7 +72,7 @@ abstract class LocalTable implements FeTable {
LocalTable t = null;
Table msTbl = schemaInfo.msTable_;
if (TableType.valueOf(msTbl.getTableType()) == TableType.VIRTUAL_VIEW) {
- // TODO(todd) support View
+ t = new LocalView(db, tblName, schemaInfo);
} else if (HBaseTable.isHBaseTable(msTbl)) {
// TODO(todd) support HBase table
} else if (KuduTable.isKuduTable(msTbl)) {
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java
new file mode 100644
index 0000000..c380eb1
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalView.java
@@ -0,0 +1,83 @@
+// 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.local;
+
+import java.util.List;
+import java.util.Set;
+
+import org.apache.impala.analysis.QueryStmt;
+import org.apache.impala.catalog.FeView;
+import org.apache.impala.catalog.TableLoadingException;
+import org.apache.impala.catalog.View;
+import org.apache.impala.thrift.TCatalogObjectType;
+import org.apache.impala.thrift.TTableDescriptor;
+
+/**
+ * View implementation corresponding to the LocalCatalog.
+ *
+ * NOTE: this does _not_ implement "local views" in the sense of a view defined
+ * in the scope of a query using a WITH expression.
+ */
+public class LocalView extends LocalTable implements FeView {
+ private final QueryStmt queryStmt_;
+
+ public LocalView(LocalDb db, String tblName, SchemaInfo schemaInfo) {
+ super(db, tblName, schemaInfo);
+
+ try {
+ queryStmt_ = View.parseViewDef(this);
+ } catch (TableLoadingException e) {
+ throw new LocalCatalogException(e);
+ }
+ }
+
+ @Override
+ public TTableDescriptor toThriftDescriptor(int tableId,
+ Set<Long> referencedPartitions) {
+ // Views are always resolved into underlying tables before being serialized.
+ throw new IllegalStateException("Cannot call toThriftDescriptor() on a view.");
+ }
+
+ // NOTE: "local view" in this context means "a view defined local to a query"
+ // whereas "local" in the class name "LocalView" means "view from the
+ // LocalCatalog catalog implementation".
+ @Override
+ public boolean isLocalView() {
+ // TODO: the org.apache.impala.catalog.View class is still used for local views
+ // even in the LocalCatalog implementation.
+ return false;
+ }
+
+ @Override
+ public QueryStmt getQueryStmt() {
+ return queryStmt_;
+ }
+
+ @Override
+ public List<String> getColLabels() {
+ // Explicit column labels are only used by local views.
+ // Returning null indicates that the column labels are derived
+ // from the underlying statement.
+ return null;
+ }
+
+ @Override
+ public TCatalogObjectType getCatalogObjectType() {
+ return TCatalogObjectType.VIEW;
+ }
+}
diff --git a/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
index e334d20..d85b8e3 100644
--- a/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java
@@ -29,9 +29,11 @@ import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.FeFsPartition;
import org.apache.impala.catalog.FeFsTable;
import org.apache.impala.catalog.FeTable;
+import org.apache.impala.catalog.FeView;
import org.apache.impala.catalog.HdfsPartition.FileDescriptor;
import org.apache.impala.catalog.Type;
import org.apache.impala.service.FeSupport;
+import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.thrift.TResultSet;
import org.apache.impala.util.MetaStoreUtil;
import org.apache.impala.util.PatternMatcher;
@@ -167,4 +169,11 @@ public class LocalCatalogTest {
assertEquals(10210, stats.getNumDistinctValues());
assertEquals(-1, stats.getNumNulls());
}
+
+ @Test
+ public void testView() throws Exception {
+ FeView v = (FeView) catalog_.getTable("functional", "alltypes_view");
+ assertEquals(TCatalogObjectType.VIEW, v.getCatalogObjectType());
+ assertEquals("SELECT * FROM functional.alltypes", v.getQueryStmt().toSql());
+ }
}
[impala] 04/06: IMPALA-7228: Add tpcds-unmodified to
single-node-perf-run
Posted by ta...@apache.org.
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 43e54501cece5e4d4a2f8c483465dd81d8b6a115
Author: njanarthanan <nj...@cloudera.com>
AuthorDate: Thu Jul 12 14:18:10 2018 -0700
IMPALA-7228: Add tpcds-unmodified to single-node-perf-run
Description:
tpcds-unmodified workload was added as a part of IMPALA-6819.
This change allows tpcds-unmodified workload to be available
for the single node perf run.
Testing:
Ran single node perf run using the following parameters and the
test run was successful
--iterations 2 --scale 2 --table_formats "parquet/none" \
--num_impalads 1 --workload "tpcds-unmodified" \
--load --query_names "TPCDS-Q17.*" --start_minicluster
Change-Id: I511661c586cd55e3240ccbea9c499b9c3fc98440
Reviewed-on: http://gerrit.cloudera.org:8080/10931
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
bin/single_node_perf_run.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bin/single_node_perf_run.py b/bin/single_node_perf_run.py
index c49e8e0..c931266 100755
--- a/bin/single_node_perf_run.py
+++ b/bin/single_node_perf_run.py
@@ -227,7 +227,8 @@ def perf_ab_test(options, args):
workloads = set(options.workloads.split(","))
if options.load:
- WORKLOAD_TO_DATASET = {"tpch": "tpch", "tpcds": "tpcds", "targeted-perf": "tpch"}
+ WORKLOAD_TO_DATASET = {"tpch": "tpch", "tpcds": "tpcds", "targeted-perf": "tpch",
+ "tpcds-unmodified": "tpcds-unmodified"}
datasets = set([WORKLOAD_TO_DATASET[workload] for workload in workloads])
for dataset in datasets:
load_data(dataset, options.table_formats, options.scale)
[impala] 03/06: IMPALA-6086: Use of permanent function should
require SELECT privilege on DB
Posted by ta...@apache.org.
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 2e720ace8b285ae6a3b6b5ebc63dcfd04a763ca1
Author: Zoram Thanga <zo...@cloudera.com>
AuthorDate: Mon Jul 2 13:43:50 2018 -0700
IMPALA-6086: Use of permanent function should require SELECT privilege
on DB
To use a permanent UDF should require at least SELECT privilege on the
database. Functions that have constant arguments get constant-folded
into string literals, losing their privilege requests in the process.
This patch saves the privilege requests found during the first phase
of query analysis, where all the objects and the privileges required
to access them are identified. The requests are added back to the
new analyzer created for re-analysis post expression rewrite.
Testing:
New FE test cases have been added to AuthorizationStmtTest.
Manual tests were also done to identify the bug, as well as to test
the fix.
Ran exhaustive and covering tests.
Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Reviewed-on: http://gerrit.cloudera.org:8080/10850
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../apache/impala/analysis/AnalysisContext.java | 13 +++++++
.../impala/analysis/AuthorizationStmtTest.java | 41 +++++++++++++++++++---
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index 5ed791e..9ad0807 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -51,6 +51,7 @@ import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.ImmutableList;
/**
* Wrapper class for parsing, analyzing and rewriting a SQL stmt.
@@ -461,6 +462,13 @@ public class AnalysisContext {
List<String> origColLabels =
Lists.newArrayList(analysisResult_.stmt_.getColLabels());
+ // Some expressions, such as function calls with constant arguments, can get
+ // folded into literals. Since literals do not require privilege requests, we
+ // must save the original privileges in order to not lose them during
+ // re-analysis.
+ ImmutableList<PrivilegeRequest> origPrivReqs =
+ analysisResult_.analyzer_.getPrivilegeReqs();
+
// Re-analyze the stmt with a new analyzer.
analysisResult_.analyzer_ = createAnalyzer(stmtTableCache);
analysisResult_.stmt_.reset();
@@ -472,6 +480,11 @@ public class AnalysisContext {
if (LOG.isTraceEnabled()) {
LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql());
}
+
+ // Restore privilege requests found during the first pass
+ for (PrivilegeRequest req : origPrivReqs) {
+ analysisResult_.analyzer_.registerPrivReq(req);
+ }
if (isExplain) analysisResult_.stmt_.setIsExplain();
Preconditions.checkState(!analysisResult_.requiresSubqueryRewrite());
}
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
index 5406f53..52e0952 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -41,6 +41,7 @@ import org.apache.impala.thrift.TFunctionBinaryType;
import org.apache.impala.thrift.TPrivilege;
import org.apache.impala.thrift.TPrivilegeLevel;
import org.apache.impala.thrift.TPrivilegeScope;
+import org.apache.impala.thrift.TQueryOptions;
import org.apache.impala.thrift.TResultRow;
import org.apache.impala.thrift.TTableName;
import org.apache.impala.util.SentryPolicyService;
@@ -2118,6 +2119,33 @@ public class AuthorizationStmtTest extends FrontendTestBase {
} finally {
removeFunction(fn);
}
+
+ // IMPALA-6086: Make sure use of a permanent function requires SELECT (or higher)
+ // privilege on the database, and expr rewrite/constant-folding preserves
+ // privilege requests for functions.
+ ArrayList<Type> argTypes = new ArrayList<Type>();
+ argTypes.add(Type.STRING);
+ fn = addFunction("functional", "to_lower", argTypes, Type.STRING,
+ "/test-warehouse/libTestUdf.so",
+ "_Z7ToLowerPN10impala_udf15FunctionContextERKNS_9StringValE");
+ try {
+ TQueryOptions options = new TQueryOptions();
+ options.setEnable_expr_rewrites(true);
+ for (AuthzTest test: new AuthzTest[] {
+ authorize("select functional.to_lower('ABCDEF')"),
+ // Also test with expression rewrite enabled.
+ authorize(createAnalysisCtx(options),
+ "select functional.to_lower('ABCDEF')")}) {
+ test.ok(onServer(TPrivilegeLevel.SELECT))
+ .ok(onDatabase("functional", TPrivilegeLevel.ALL))
+ .ok(onDatabase("functional", viewMetadataPrivileges()))
+ .error(accessError("functional"))
+ .error(accessError("functional"), onDatabase("functional",
+ allExcept(viewMetadataPrivileges())));
+ }
+ } finally {
+ removeFunction(fn);
+ }
}
// Convert TDescribeResult to list of strings.
@@ -2172,14 +2200,19 @@ public class AuthorizationStmtTest extends FrontendTestBase {
return "User '%s' does not have privileges to DROP functions in: " + object;
}
- private ScalarFunction addFunction(String db, String fnName) {
- ScalarFunction fn = ScalarFunction.createForTesting(db, fnName,
- new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
- null, TFunctionBinaryType.NATIVE);
+ private ScalarFunction addFunction(String db, String fnName, ArrayList<Type> argTypes,
+ Type retType, String uriPath, String symbolName) {
+ ScalarFunction fn = ScalarFunction.createForTesting(db, fnName, argTypes, retType,
+ uriPath, symbolName, null, null, TFunctionBinaryType.NATIVE);
authzCatalog_.addFunction(fn);
return fn;
}
+ private ScalarFunction addFunction(String db, String fnName) {
+ return addFunction(db, fnName, new ArrayList<Type>(), Type.INT, "/dummy",
+ "dummy.class");
+ }
+
private void removeFunction(ScalarFunction fn) {
authzCatalog_.removeFunction(fn);
}
[impala] 05/06: IMPALA-6810: runtime_row_filters.test: omit pool
name in pattern
Posted by ta...@apache.org.
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 d0a6239be30b64f1193394236de0564bebe9f696
Author: Michael Brown <mi...@cloudera.com>
AuthorDate: Thu Jul 12 10:40:26 2018 -0700
IMPALA-6810: runtime_row_filters.test: omit pool name in pattern
Some downstream tests run this with a fair-scheduler.xml set that, while
not changing admission control behavior, does change the name of the
pool. Omit the pool name to permit that downstream test to succeed.
Testing:
- local with change in minicluster
- downstream in environment as well
Change-Id: I3fe6beb169dc6bfefabde9dc7a4632c1a5e63fa7
Reviewed-on: http://gerrit.cloudera.org:8080/10942
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../functional-query/queries/QueryTest/runtime_row_filters.test | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test b/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
index fe61741..ff13999 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
@@ -401,10 +401,9 @@ select STRAIGHT_JOIN * from alltypes a join [SHUFFLE] alltypes b
on a.month = b.id and b.int_col = -3
---- RESULTS
---- CATCH
-row_regex:.*Rejected query from pool default-pool: minimum memory reservation on
- backend '.*' is greater than memory available to the query for buffer reservations\.
- Increase the buffer_pool_limit to 290.17 MB\. See the query profile for more information
- about the per-node memory requirements\.
+row_regex:.*minimum memory reservation on backend '.*' is greater than memory available to
+ the query for buffer reservations\. Increase the buffer_pool_limit to 290.17 MB\. See
+ the query profile for more information about the per-node memory requirements\.
====
---- QUERY
# Confirm that with broadcast join, memory limit is not hit.
[impala] 06/06: IMPALA-7288: Fix Codegen Crash in FinalizeModule()
(Addendum)
Posted by ta...@apache.org.
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 aea18dd08f34caf5c659c4b71f7bc4d70d743739
Author: Bikramjeet Vig <bi...@cloudera.com>
AuthorDate: Fri Jul 13 13:07:54 2018 -0700
IMPALA-7288: Fix Codegen Crash in FinalizeModule() (Addendum)
In addition to previous fix for IMPALA-7288, this patch would prevent
impala from crashing in case a code-path generates a malformed
handcrafted function which it then tries to finalize. Ideally this
would never happen since the code paths for generating handcrafted IRs
would never generate a malformed function.
Change-Id: Id09c6f59f677ba30145fb2081715f1a7d89fe20b
Reviewed-on: http://gerrit.cloudera.org:8080/10944
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/codegen/llvm-codegen.cc | 5 +----
be/src/codegen/llvm-codegen.h | 4 ++--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index 7fe4ec1..173dbb2 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -1009,10 +1009,7 @@ llvm::Function* LlvmCodeGen::CloneFunction(llvm::Function* fn) {
llvm::Function* LlvmCodeGen::FinalizeFunction(llvm::Function* function) {
SetCPUAttrs(function);
- if (!VerifyFunction(function)) {
- function->eraseFromParent(); // deletes function
- return NULL;
- }
+ if (!VerifyFunction(function)) return NULL;
finalized_functions_.insert(function);
if (FLAGS_dump_ir) {
string fn_name = function->getName();
diff --git a/be/src/codegen/llvm-codegen.h b/be/src/codegen/llvm-codegen.h
index 53569ca..7e9da26 100644
--- a/be/src/codegen/llvm-codegen.h
+++ b/be/src/codegen/llvm-codegen.h
@@ -392,8 +392,8 @@ class LlvmCodeGen {
/// passed to AddFunctionToJit() otherwise the functions will be deleted from the
/// module when the module is finalized. Also, all loaded functions that need to be JIT
/// compiled after modification also need to be finalized.
- /// If the function does not verify, it will delete the function and return NULL,
- /// otherwise, it returns the function object.
+ /// If the function does not verify, it returns NULL and the function will eventually
+ /// be deleted in FinalizeModule(), otherwise, it returns the function object.
llvm::Function* FinalizeFunction(llvm::Function* function);
/// Adds the function to be automatically jit compiled when the codegen object is