You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2023/12/11 18:47:11 UTC
(impala) branch master updated (d9c067aa8 -> 8874ea07b)
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from d9c067aa8 IMPALA-12398: Fix Ranger role not exists when altering db/table/view owner to a role
new 4c28ab02c IMPALA-12001: Informative error message for complex types with DISTINCT
new 8874ea07b IMPALA-11805: Fix LLVM memory manager bytes allocated
The 2 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-cache-test.cc | 30 ++++++++++++----------
be/src/codegen/llvm-codegen-test.cc | 6 +++++
be/src/codegen/mcjit-mem-mgr.h | 10 ++------
be/src/thirdparty/llvm/SectionMemoryManager.cpp | 10 ++++++++
be/src/thirdparty/llvm/SectionMemoryManager.h | 6 +++++
.../apache/impala/analysis/MultiAggregateInfo.java | 16 ++++++++++++
.../org/apache/impala/analysis/SelectStmt.java | 6 +++++
.../QueryTest/nested-array-in-select-list.test | 12 ++++++++-
.../QueryTest/nested-map-in-select-list.test | 18 ++++++++++---
.../QueryTest/nested-struct-in-select-list.test | 13 ++++++++--
10 files changed, 98 insertions(+), 29 deletions(-)
(impala) 02/02: IMPALA-11805: Fix LLVM memory manager bytes allocated
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 8874ea07b68c09cee3c4e300b70706f3c5ea40af
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Fri Nov 10 08:14:43 2023 -0800
IMPALA-11805: Fix LLVM memory manager bytes allocated
Previously the bytes_allocated function would undercount memory
allocated by the LLVM memory manager, as it allocates memory in full
pages but bytes_allocated only tracked the actual storage requested.
Implements bytes_allocated in the SectionMemoryManager to access the
actual storage size allocated.
Change-Id: I5a0193a1694db0718d75651dc2b2f9f92c0d6064
Reviewed-on: http://gerrit.cloudera.org:8080/20697
Reviewed-by: Yida Wu <wy...@gmail.com>
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
be/src/codegen/llvm-codegen-cache-test.cc | 30 +++++++++++++------------
be/src/codegen/llvm-codegen-test.cc | 6 +++++
be/src/codegen/mcjit-mem-mgr.h | 10 ++-------
be/src/thirdparty/llvm/SectionMemoryManager.cpp | 10 +++++++++
be/src/thirdparty/llvm/SectionMemoryManager.h | 6 +++++
5 files changed, 40 insertions(+), 22 deletions(-)
diff --git a/be/src/codegen/llvm-codegen-cache-test.cc b/be/src/codegen/llvm-codegen-cache-test.cc
index e1976eeca..d8be5a399 100644
--- a/be/src/codegen/llvm-codegen-cache-test.cc
+++ b/be/src/codegen/llvm-codegen-cache-test.cc
@@ -18,6 +18,7 @@
#include "codegen/llvm-codegen-cache.h"
#include <boost/thread/thread.hpp>
#include <llvm/ExecutionEngine/ExecutionEngine.h>
+#include "llvm/Support/Process.h"
#include "codegen/mcjit-mem-mgr.h"
#include "common/object-pool.h"
#include "runtime/fragment-state.h"
@@ -36,6 +37,9 @@ namespace impala {
class LlvmCodeGenCacheTest : public testing::Test {
public:
virtual void SetUp() {
+ // Using single shard makes the logic of scenarios simple for capacity and
+ // eviction-related behavior.
+ FLAGS_cache_force_single_shard = true;
FLAGS_codegen_cache_capacity = "0";
metrics_.reset(new MetricGroup("codegen-cache-test"));
profile_ = RuntimeProfile::Create(&obj_pool_, "codegen-cache-test");
@@ -48,9 +52,11 @@ class LlvmCodeGenCacheTest : public testing::Test {
PlanFragmentCtxPB* fragment_ctx = qs->obj_pool()->Add(new PlanFragmentCtxPB());
fragment_state_ =
qs->obj_pool()->Add(new FragmentState(qs, *fragment, *fragment_ctx));
+ page_size_ = llvm::sys::Process::getPageSize();
}
virtual void TearDown() {
+ FLAGS_cache_force_single_shard = false;
fragment_state_->ReleaseResources();
fragment_state_ = nullptr;
codegen_cache_.reset();
@@ -137,6 +143,7 @@ class LlvmCodeGenCacheTest : public testing::Test {
shared_ptr<LlvmExecutionEngineWrapper> exec_engine_wrapper_;
TQueryOptions query_options_;
TCodeGenOptLevel::type opt_level_ = TCodeGenOptLevel::O2;
+ unsigned page_size_;
};
void LlvmCodeGenCacheTest::AddLlvmCodegenEcho(LlvmCodeGen* codegen) {
@@ -268,19 +275,10 @@ int64_t LlvmCodeGenCacheTest::GetMemCharge(
/// Test the situation that the codegen cache hits the limit of capacity, in this case,
/// eviction is needed when new insertion comes.
void LlvmCodeGenCacheTest::TestAtCapacity(TCodeGenCacheMode::type mode) {
- int64_t codegen_cache_capacity = 196; // 196B
+ // LLVM allocates memory in pages, and for small examples allocates 3 pages.
+ int64_t codegen_cache_capacity = 3 * page_size_ +
+ sizeof(CodeGenCacheEntry) + CodeGenCacheKey::OptimalKeySize + sizeof(std::string);
bool is_normal_mode = !CodeGenCacheModeAnalyzer::is_optimal(mode);
- // 150B for optimal mode, or 164B on ARM systems
- if (!is_normal_mode) {
-#ifdef __aarch64__
- codegen_cache_capacity = 164;
-#else
- codegen_cache_capacity = 150;
-#endif
- }
- // Using single shard makes the logic of scenarios simple for capacity and
- // eviction-related behavior.
- FLAGS_cache_force_single_shard = true;
// Create two LlvmCodeGen objects containing a different codegen function separately.
scoped_ptr<LlvmCodeGen> codegen;
@@ -488,7 +486,9 @@ void LlvmCodeGenCacheTest::TestSwitchModeHelper(TCodeGenCacheMode::type mode, st
// Test to switch among different modes.
TEST_F(LlvmCodeGenCacheTest, SwitchMode) {
- int64_t codegen_cache_capacity = 512; // 512B
+ // Storage for 2 entries
+ int64_t codegen_cache_capacity = 2 * (3 * page_size_ +
+ sizeof(CodeGenCacheEntry) + CodeGenCacheKey::OptimalKeySize + sizeof(std::string));
codegen_cache_.reset(new CodeGenCache(metrics_.get()));
EXPECT_OK(codegen_cache_->Init(codegen_cache_capacity));
string key = "key";
@@ -579,7 +579,9 @@ void LlvmCodeGenCacheTest::TestConcurrentStore(int num_threads) {
}
TEST_F(LlvmCodeGenCacheTest, ConcurrentStore) {
- int64_t codegen_cache_capacity = 512; // 512B
+ // Storage for 2 entries
+ int64_t codegen_cache_capacity = 2 * (3 * page_size_ +
+ sizeof(CodeGenCacheEntry) + CodeGenCacheKey::OptimalKeySize + sizeof(std::string));
codegen_cache_.reset(new CodeGenCache(metrics_.get()));
EXPECT_OK(codegen_cache_->Init(codegen_cache_capacity));
TestConcurrentStore(8);
diff --git a/be/src/codegen/llvm-codegen-test.cc b/be/src/codegen/llvm-codegen-test.cc
index 3f6f70daa..2b87267a0 100644
--- a/be/src/codegen/llvm-codegen-test.cc
+++ b/be/src/codegen/llvm-codegen-test.cc
@@ -39,6 +39,8 @@
using std::unique_ptr;
+DECLARE_bool(cache_force_single_shard);
+
namespace impala {
class CodegenFnPtrTest : public testing:: Test {
@@ -579,6 +581,9 @@ class LlvmOptTest :
TQueryOptions query_opts_;
virtual void SetUp() {
+ // Using single shard makes the logic of scenarios simple for capacity and
+ // eviction-related behavior.
+ FLAGS_cache_force_single_shard = true;
metrics_.reset(new MetricGroup("codegen-test"));
test_env_.reset(new TestEnv());
ASSERT_OK(test_env_->Init());
@@ -586,6 +591,7 @@ class LlvmOptTest :
}
virtual void TearDown() {
+ FLAGS_cache_force_single_shard = false;
fragment_state_->ReleaseResources();
fragment_state_ = nullptr;
test_env_.reset();
diff --git a/be/src/codegen/mcjit-mem-mgr.h b/be/src/codegen/mcjit-mem-mgr.h
index 6002b6087..5844cb6f7 100644
--- a/be/src/codegen/mcjit-mem-mgr.h
+++ b/be/src/codegen/mcjit-mem-mgr.h
@@ -36,7 +36,7 @@ namespace impala {
/// We also use it to track how much memory is allocated for compiled code.
class ImpalaMCJITMemoryManager : public SectionMemoryManager {
public:
- ImpalaMCJITMemoryManager() : bytes_allocated_(0), bytes_tracked_(0){}
+ ImpalaMCJITMemoryManager() : bytes_tracked_(0) {}
virtual uint64_t getSymbolAddress(const std::string& name) override {
if (name == "__dso_handle") return reinterpret_cast<uint64_t>(&__dso_handle);
@@ -45,29 +45,23 @@ class ImpalaMCJITMemoryManager : public SectionMemoryManager {
virtual uint8_t* allocateCodeSection(uintptr_t size, unsigned alignment,
unsigned section_id, llvm::StringRef section_name) override {
- bytes_allocated_ += size;
return SectionMemoryManager::allocateCodeSection(
size, alignment, section_id, section_name);
}
virtual uint8_t* allocateDataSection(uintptr_t size, unsigned alignment,
unsigned section_id, llvm::StringRef section_name, bool is_read_only) override {
- bytes_allocated_ += size;
return SectionMemoryManager::allocateDataSection(
size, alignment, section_id, section_name, is_read_only);
}
- int64_t bytes_allocated() const { return bytes_allocated_; }
int64_t bytes_tracked() const { return bytes_tracked_; }
void set_bytes_tracked(int64_t bytes_tracked) {
- DCHECK_LE(bytes_tracked, bytes_allocated_);
+ DCHECK_LE(bytes_tracked, bytes_allocated());
bytes_tracked_ = bytes_tracked;
}
private:
- /// Total bytes allocated for the compiled code.
- int64_t bytes_allocated_;
-
/// Total bytes already tracked by MemTrackers. <= 'bytes_allocated_'.
/// Needed to release the correct amount from the MemTracker when done.
int64_t bytes_tracked_;
diff --git a/be/src/thirdparty/llvm/SectionMemoryManager.cpp b/be/src/thirdparty/llvm/SectionMemoryManager.cpp
index 5964275ad..825843c0a 100644
--- a/be/src/thirdparty/llvm/SectionMemoryManager.cpp
+++ b/be/src/thirdparty/llvm/SectionMemoryManager.cpp
@@ -311,6 +311,16 @@ void SectionMemoryManager::invalidateInstructionCache() {
sys::Memory::InvalidateInstructionCache(Block.base(), Block.size());
}
+// Impala: added to track actual allocation
+int64_t SectionMemoryManager::bytes_allocated() const {
+ int64_t total = 0;
+ for (const MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
+ for (const sys::MemoryBlock &Block : Group->AllocatedMem)
+ total += Block.size();
+ }
+ return total;
+}
+
SectionMemoryManager::~SectionMemoryManager() {
for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
for (sys::MemoryBlock &Block : Group->AllocatedMem)
diff --git a/be/src/thirdparty/llvm/SectionMemoryManager.h b/be/src/thirdparty/llvm/SectionMemoryManager.h
index 23ba5de28..fdfcb917a 100644
--- a/be/src/thirdparty/llvm/SectionMemoryManager.h
+++ b/be/src/thirdparty/llvm/SectionMemoryManager.h
@@ -105,6 +105,12 @@ public:
/// This method is called from finalizeMemory.
virtual void invalidateInstructionCache();
+ /// \brief Returns number of bytes allocated by the memory manager.
+ ///
+ /// Impala: added to track actual memory allocation (which is page-aligned)
+ /// rather than just requested memory.
+ int64_t bytes_allocated() const;
+
private:
struct FreeMemBlock {
// The actual block of free memory
(impala) 01/02: IMPALA-12001: Informative error message for complex types with DISTINCT
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 4c28ab02c658b559c47cf9993448218e228d61c9
Author: Daniel Becker <da...@cloudera.com>
AuthorDate: Tue Dec 5 15:00:15 2023 +0100
IMPALA-12001: Informative error message for complex types with DISTINCT
Before this change, queries with SELECT DISTINCT on a complex type
failed.
With structs, we got a FE exception:
use functional_parquet;
select distinct(struct_val) from alltypes_structs;
ERROR: IllegalStateException: null
With collections, the BE hits a DCHECK and crashes:
use functional_parquet;
select distinct(arr1) from complextypes_arrays;
Socket error 104: [Errno 104] Connection reset by peer
Aggregate functions with complex DISTINCT parameters also failed without
a clear error message. For example:
select count(distinct struct_val) from alltypes_structs;
select count(distinct arr1) from complextypes_arrays;
To support DISTINCT for complex types we would need to implement
equality and hash for them. We are not planning to do it in the near
future, so this change introduces informative error messages in these
cases.
Testing:
- added test queries for SELECT DISTINCT and SELECT COUNT(DISTINCT ...)
with arrays, maps and structs, expecting the correct error messages.
Change-Id: Ibe2642d1683a10fd05a95e2ad8470d16f0d5242c
Reviewed-on: http://gerrit.cloudera.org:8080/20752
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../org/apache/impala/analysis/MultiAggregateInfo.java | 16 ++++++++++++++++
.../java/org/apache/impala/analysis/SelectStmt.java | 6 ++++++
.../queries/QueryTest/nested-array-in-select-list.test | 12 +++++++++++-
.../queries/QueryTest/nested-map-in-select-list.test | 18 ++++++++++++++----
.../QueryTest/nested-struct-in-select-list.test | 13 +++++++++++--
5 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java b/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
index 48cb37f96..24c12fda9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
+++ b/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
@@ -249,6 +249,10 @@ public class MultiAggregateInfo {
groupingBuiltinExprs.add(aggExpr);
} else if (aggExpr.isDistinct()) {
List<Expr> children = AggregateFunction.getCanonicalDistinctAggChildren(aggExpr);
+
+ // Complex types are not supported as DISTINCT parameters of aggregate functions.
+ checkComplexDistinctParams(aggExpr, children);
+
int groupIdx = distinctExprs.indexOf(children);
List<FunctionCallExpr> groupAggFns;
if (groupIdx == -1) {
@@ -342,6 +346,18 @@ public class MultiAggregateInfo {
}
}
+ private static void checkComplexDistinctParams(FunctionCallExpr aggExpr,
+ List<Expr> params) throws AnalysisException {
+ for (Expr child : params) {
+ if (child.getType().isComplexType()) {
+ throw new AnalysisException("Complex types are not supported " +
+ "as DISTINCT parameters of aggregate functions. Distinct parameter: '" +
+ child.toSql() + "', type: '" + child.getType().toSql() +
+ "' in aggregate function '" + aggExpr.toSql() + "'.");
+ }
+ }
+ }
+
/**
* Implementation of analyze() for aggregation with grouping sets.
* Does not handle distinct aggregate functions yet.
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index 25a79c189..48c123759 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -562,6 +562,12 @@ public class SelectStmt extends QueryStmt {
}
for (Expr expr: resultExprs_) {
+ if (selectList_.isDistinct() && expr.getType().isComplexType()) {
+ throw new AnalysisException("Complex types are not supported " +
+ "in SELECT DISTINCT clauses. Expr: '" + expr.toSql() + "', type: '"
+ + expr.getType().toSql() + "'.");
+ }
+
if (expr.getType().isArrayType()) {
ArrayType arrayType = (ArrayType) expr.getType();
if (!arrayType.getItemType().isSupported()) {
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test b/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test
index 5c84778bd..4d4abc5ab 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test
@@ -425,4 +425,14 @@ on a.id = b.id where a.id < 3;
0,'NULL','NULL'
---- TYPES
INT,STRING,STRING
-=====
+====
+---- QUERY
+select distinct arr1 from complextypes_arrays
+---- CATCH
+AnalysisException: Complex types are not supported in SELECT DISTINCT clauses.
+====
+---- QUERY
+select count(distinct arr1) from complextypes_arrays
+---- CATCH
+AnalysisException: Complex types are not supported as DISTINCT parameters of aggregate functions.
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-map-in-select-list.test b/testdata/workloads/functional-query/queries/QueryTest/nested-map-in-select-list.test
index 337dfbd55..aedebd289 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-map-in-select-list.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-map-in-select-list.test
@@ -392,7 +392,7 @@ select id, map_1d, map_2d, map_3d, arr_int_3d, map_map_array from collection_tbl
3,'{645:"fourth even-toed ungulate",5:"fifth"}','{1:{10:"ten",20:"twentieth even-toed ungulate"},2:{30:"thirty even-toed ungulates",40:"forty"}}','{1:{10:{100:"hundred",200:"two hundred even-toed ungulates"},20:{300:"three hundred even-toed ungulates",400:"four hundred"}},2:{30:{500:"five hundred even-toed ungulates",600:"six hundred"},40:{700:"seven hundred even-toed ungulates",800:"eight hundred"}}}','[[[1,null,2,null],[null,15]],[[null,4]]]','{1:{10:[100,200],20:[300,400]},2:{30:[500, [...]
---- TYPES
INT,STRING,STRING,STRING,STRING,STRING
-=====
+====
---- QUERY
select id, map_1d, map_2d, mma.value mma_value, ma.value ma_value
from collection_tbl c, c.map_map_array mma, mma.value ma;
@@ -411,7 +411,7 @@ from collection_tbl c, c.map_map_array mma, mma.value ma;
3,'{645:"fourth even-toed ungulate",5:"fifth"}','{1:{10:"ten",20:"twentieth even-toed ungulate"},2:{30:"thirty even-toed ungulates",40:"forty"}}','{30:[500,600],40:[700,800]}','[700,800]'
---- TYPES
INT,STRING,STRING,STRING,STRING
-=====
+====
---- QUERY
-- Test that map keys are printed correctly.
set CONVERT_LEGACY_HIVE_PARQUET_UTC_TIMESTAMPS=1;
@@ -435,7 +435,7 @@ from collection_tbl;
'{true:"true even-toed ungulate",false:"false"}','{-1:"a nice even-toed ungulate",0:"best even-toed ungulate",1:"c"}','{-1:"a nice even-toed ungulate",0:"best even-toed ungulate",1:"c"}','{-1:"a nice even-toed ungulate",0:"best even-toed ungulate",1:"c"}','{-1.5:"a nice even-toed ungulate",0.25:"best even-toed ungulate",1.75:"c"}','{-1.5:"a nice even-toed ungulate",0.25:"best even-toed ungulate",1.75:"c"}','{-1.8:"a nice even-toed ungulate",0.2:"best even-toed ungulate",1.2:"c"}','{"one" [...]
---- TYPES
STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING
-=====
+====
---- QUERY
-- Test that map keys are printed correctly with STRINGIFY_MAP_KEYS=true.
set CONVERT_LEGACY_HIVE_PARQUET_UTC_TIMESTAMPS=1;
@@ -460,4 +460,14 @@ from collection_tbl;
'{"true":"true even-toed ungulate","false":"false"}','{"-1":"a nice even-toed ungulate","0":"best even-toed ungulate","1":"c"}','{"-1":"a nice even-toed ungulate","0":"best even-toed ungulate","1":"c"}','{"-1":"a nice even-toed ungulate","0":"best even-toed ungulate","1":"c"}','{"-1.5":"a nice even-toed ungulate","0.25":"best even-toed ungulate","1.75":"c"}','{"-1.5":"a nice even-toed ungulate","0.25":"best even-toed ungulate","1.75":"c"}','{"-1.8":"a nice even-toed ungulate","0.2":"best [...]
---- TYPES
STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING,STRING
-=====
+====
+---- QUERY
+select distinct int_map from complextypestbl
+---- CATCH
+AnalysisException: Complex types are not supported in SELECT DISTINCT clauses.
+====
+---- QUERY
+select count(distinct int_map) from complextypestbl;
+---- CATCH
+AnalysisException: Complex types are not supported as DISTINCT parameters of aggregate functions.
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test b/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
index 846c78d2f..e00500632 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/nested-struct-in-select-list.test
@@ -452,8 +452,7 @@ from complextypes_nested_structs;
INT,STRING
====
---- QUERY
-# Subquery that returns a complex type is not supported.
-# IMPALA-9500
+# Complex types in IN predicates are not supported.
select outer_struct
from complextypes_nested_structs
where outer_struct in
@@ -462,3 +461,13 @@ where outer_struct in
AnalysisException: A subquery can't return complex types. (SELECT outer_struct FROM functional_parquet.complex
types_nested_structs)
====
+---- QUERY
+select distinct outer_struct from complextypes_nested_structs
+---- CATCH
+AnalysisException: Complex types are not supported in SELECT DISTINCT clauses.
+====
+---- QUERY
+select count(distinct outer_struct) from complextypes_nested_structs;
+---- CATCH
+AnalysisException: Complex types are not supported as DISTINCT parameters of aggregate functions.
+====