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 2020/11/13 03:47:14 UTC
[impala] branch master updated (5e38f32 -> 5a00a4c)
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from 5e38f32 IMPALA-10315: Update 'ICEBERG' InputFormat/OutputFormat/SerDe from latest Iceberg
new 1fd5e42 IMPALA-10310: Fix couldn't skip rows in parquet file on NextRowGroup
new 746c968 IMPALA-6861: Fix OpenSSL initialization
new 5a00a4c IMPALA-10320: Specify expression selectivity for BoolLiteral.
The 3 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/exec/parquet/parquet-column-readers.cc | 1 +
be/src/rpc/authentication.cc | 6 +++++-
be/src/rpc/authentication.h | 10 +---------
be/src/rpc/thrift-server.cc | 5 +++++
.../java/org/apache/impala/analysis/BoolLiteral.java | 3 +++
.../java/org/apache/impala/analysis/NullLiteral.java | 1 +
.../apache/impala/analysis/ExprCardinalityTest.java | 9 +++++++--
testdata/data/README | 15 +++++++++++++++
testdata/data/customer_multiblock_page_index.parquet | Bin 0 -> 451607 bytes
.../queries/QueryTest/parquet-page-index.test | 16 ++++++++++++++++
tests/query_test/test_parquet_stats.py | 2 ++
11 files changed, 56 insertions(+), 12 deletions(-)
create mode 100644 testdata/data/customer_multiblock_page_index.parquet
[impala] 01/03: IMPALA-10310: Fix couldn't skip rows in parquet
file on NextRowGroup
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 1fd5e4279c75a7cb3e51e737d9ca7c36435412dc
Author: guojingfeng <gu...@tencent.com>
AuthorDate: Thu Nov 5 03:30:33 2020 +0000
IMPALA-10310: Fix couldn't skip rows in parquet file on NextRowGroup
In practice we recommend that hdfs block size should align with parquet
row group size.But in fact some compute engine like spark, default
parquet row group size is 128MB, and if ETL user doesn't change the
default property spark will generate row groups that smaller than hdfs
block size. The result is a single hdfs block may contain multiple
parquet row groups.
In planner stage, length of impala generated scan range may be bigger
than row group size. thus a single scan range contains multiple row
group. In current parquet scanner when move to next row group, some of
internal stat in parquet column readers need to reset.
eg: num_buffered_values_, column chunk metadata, reset internal stat of
column chunk readers. But current_row_range_ offset is not reset
currently, this will cause errors
"Couldn't skip rows in file hdfs://xxx" as IMPALA-10310 points out.
This patch simply reset current_row_range_ to 0 when moving into next
row group in parquet column readers. Fix the bug IMPALA-10310.
Testing:
* Add e2e test for parquet multi blocks per file and multi pages
per block
* Ran all core tests offline.
* Manually tested all cases encountered in my production environment.
Change-Id: I964695cd53f5d5fdb6485a85cd82e7a72ca6092c
Reviewed-on: http://gerrit.cloudera.org:8080/16697
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exec/parquet/parquet-column-readers.cc | 1 +
testdata/data/README | 15 +++++++++++++++
testdata/data/customer_multiblock_page_index.parquet | Bin 0 -> 451607 bytes
.../queries/QueryTest/parquet-page-index.test | 16 ++++++++++++++++
tests/query_test/test_parquet_stats.py | 2 ++
5 files changed, 34 insertions(+)
diff --git a/be/src/exec/parquet/parquet-column-readers.cc b/be/src/exec/parquet/parquet-column-readers.cc
index b00ba51..b3030d7 100644
--- a/be/src/exec/parquet/parquet-column-readers.cc
+++ b/be/src/exec/parquet/parquet-column-readers.cc
@@ -1225,6 +1225,7 @@ void BaseScalarColumnReader::ResetPageFiltering() {
candidate_page_idx_ = -1;
current_row_ = -1;
levels_readahead_ = false;
+ current_row_range_ = 0;
}
Status BaseScalarColumnReader::StartPageFiltering() {
diff --git a/testdata/data/README b/testdata/data/README
index 3954d4b..5d24b22 100644
--- a/testdata/data/README
+++ b/testdata/data/README
@@ -592,3 +592,18 @@ int64_t r = random(); if (r % 2 + r % 3 + r % 5 == 0) return true;
Also modified HdfsParquetTableWriter::BaseColumnWriter::Flush to randomly invalidate
the offset index:
if (r ... ) location.offset = -1;
+
+customer_multiblock_page_index.parquet
+Parquet file that contains multiple blocks in a single file Needed to test IMPALA-10310.
+In order to generate this file, execute the following instruments:
+// use 1.11.0 to generate page index
+1. export HIVE_AUX_JARS_PATH=/path/parquet-hadoop-1.11.0.jar
+// in hive shell
+2. SET parquet.block.size=8192; // use little block size
+3. SET parquet.page.row.count.limit=10; // little page row count generate multi pages
+4. CREATE TABLE customer_multiblock_page_index_6
+ STORED AS PARQUET
+ TBLPROPERTIES('parquet.compression'='SNAPPY')
+ AS SELECT * FROM tpcds.customer
+ WHERE c_current_cdemo_sk IS NOT NULL ORDER BY c_current_cdemo_sk LIMIT 2000;
+generated file will contains multi blocks, multi pages per block.
diff --git a/testdata/data/customer_multiblock_page_index.parquet b/testdata/data/customer_multiblock_page_index.parquet
new file mode 100644
index 0000000..21fa9a5
Binary files /dev/null and b/testdata/data/customer_multiblock_page_index.parquet differ
diff --git a/testdata/workloads/functional-query/queries/QueryTest/parquet-page-index.test b/testdata/workloads/functional-query/queries/QueryTest/parquet-page-index.test
index 7c44e21..d3cbd9f 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/parquet-page-index.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/parquet-page-index.test
@@ -300,3 +300,19 @@ BIGINT
---- RUNTIME_PROFILE
aggregation(SUM, NumStatsFilteredPages): 0
====
+---- QUERY
+# Query table with multi blocks in a single file
+select c_birth_country,count(distinct c_customer_id) from customer_multiblock_page_index
+where c_current_cdemo_sk < 100 group by c_birth_country;
+---- RESULTS
+'SLOVAKIA',1
+'BRUNEI DARUSSALAM',1
+'BURKINA FASO',1
+'SIERRA LEONE',1
+'PORTUGAL',1
+---- TYPES
+STRING, BIGINT
+---- RUNTIME_PROFILE
+aggregation(SUM, NumPages): 30
+aggregation(SUM, NumStatsFilteredPages): 27
+====
diff --git a/tests/query_test/test_parquet_stats.py b/tests/query_test/test_parquet_stats.py
index 5087fa2..319dd1a 100644
--- a/tests/query_test/test_parquet_stats.py
+++ b/tests/query_test/test_parquet_stats.py
@@ -88,6 +88,8 @@ class TestParquetStats(ImpalaTestSuite):
create_table_from_parquet(self.client, unique_database, 'alltypes_tiny_pages_plain')
create_table_from_parquet(self.client, unique_database, 'alltypes_empty_pages')
create_table_from_parquet(self.client, unique_database, 'alltypes_invalid_pages')
+ create_table_from_parquet(self.client, unique_database,
+ 'customer_multiblock_page_index')
for batch_size in [1]:
new_vector.get_value('exec_option')['batch_size'] = batch_size
[impala] 03/03: IMPALA-10320: Specify expression selectivity for
BoolLiteral.
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 5a00a4c06f8ec40a8867dcbc036cf5bb47b8a3be
Author: Shant Hovsepian <sh...@cloudera.com>
AuthorDate: Thu Nov 12 01:56:52 2020 -0500
IMPALA-10320: Specify expression selectivity for BoolLiteral.
Setting the selectivity of true and false literals to 1 and 0
respectively.
Also setting a NullLiteral's selectivity to 0.
Testing:
- New tests in ExprCardinalityTest
Change-Id: I999942c96cd685f22d8634509d9f488f2a1f82b8
Reviewed-on: http://gerrit.cloudera.org:8080/16714
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java | 3 +++
fe/src/main/java/org/apache/impala/analysis/NullLiteral.java | 1 +
.../java/org/apache/impala/analysis/ExprCardinalityTest.java | 9 +++++++--
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java b/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
index 79ccc22..3b06e05 100644
--- a/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
@@ -30,6 +30,7 @@ public class BoolLiteral extends LiteralExpr {
public BoolLiteral(boolean value) {
this.value_ = value;
+ this.selectivity_ = value ? 1 : 0;
type_ = Type.BOOLEAN;
}
@@ -37,8 +38,10 @@ public class BoolLiteral extends LiteralExpr {
type_ = Type.BOOLEAN;
if (value.toLowerCase().equals("true")) {
this.value_ = true;
+ this.selectivity_ = 1;
} else if (value.toLowerCase().equals("false")) {
this.value_ = false;
+ this.selectivity_ = 0;
} else {
throw new AnalysisException("invalid BOOLEAN literal: " + value);
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java b/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
index 65ceefe..9a2f1e3 100644
--- a/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
+++ b/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
@@ -28,6 +28,7 @@ public class NullLiteral extends LiteralExpr {
public NullLiteral() {
type_ = Type.NULL;
+ this.selectivity_ = 0;
}
/**
diff --git a/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java b/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java
index deae557..0e517a5 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ExprCardinalityTest.java
@@ -238,8 +238,9 @@ public class ExprCardinalityTest {
// Note that the constant NULL has an NDV = 1, but
// Null-only columns have an NDV=0...
// See IMPALA-8058
- verifySelectExpr("alltypes", "NULL", 1, -1);
- verifySelectExpr("alltypes", "true", 1, -1);
+ verifySelectExpr("alltypes", "NULL", 1, 0);
+ verifySelectExpr("alltypes", "true", 1, 1);
+ verifySelectExpr("alltypes", "false", 1, 0);
}
// Expression selectivity
@@ -540,6 +541,10 @@ public class ExprCardinalityTest {
3, 1.0/2 + 1.0/10 - 1.0/2 * 1.0/10);
// Chain of OR rewritten to IN
verifySelectExpr("alltypes", "int_col = 10 or int_col = 20", 3, 2.0/10);
+ // Or with literals
+ verifySelectExpr("alltypes", "int_col = 10 or true", 3, 1.0);
+ verifySelectExpr("alltypes", "int_col = 10 or false", 3, 0.1);
+ verifySelectExpr("alltypes", "int_col = 10 or null", 3, 0.1);
}
/**
[impala] 02/03: IMPALA-6861: Fix OpenSSL initialization
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 746c968ac372938d64298372a0790ec18135c889
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Tue Nov 3 14:22:08 2020 -0800
IMPALA-6861: Fix OpenSSL initialization
Impalads currently initialize OpenSSL twice: once when initializing
Thrift and once when initializing KRPC. The initialization is
theoretically idempotent but not thread-safe, so its better to clean
this up.
This patch disables the Thrift version as its older (last updated in
2015) and the KRPC version contains logic specific to more recent
versions of OpenSSL. The catalogd and statestored also now use the
KRPC version instead of the Thrift version.
It also improves debuggability by adding some additional startup
logging.
Testing:
- Passed existing SSL tests.
Change-Id: I35b1362d40c8a12082cc8b531a38b4a485bac0e7
Reviewed-on: http://gerrit.cloudera.org:8080/16704
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/rpc/authentication.cc | 6 +++++-
be/src/rpc/authentication.h | 10 +---------
be/src/rpc/thrift-server.cc | 5 +++++
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index 1b3f82c..1d7acca 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -43,6 +43,7 @@
#include "kudu/rpc/sasl_common.h"
#include "kudu/security/gssapi.h"
#include "kudu/security/init.h"
+#include "kudu/security/openssl_util.h"
#include "rpc/auth-provider.h"
#include "rpc/authentication-util.h"
#include "rpc/thrift-server.h"
@@ -1112,7 +1113,10 @@ AuthManager::AuthManager() {}
AuthManager::~AuthManager() {}
Status AuthManager::Init() {
- ssl_socket_factory_.reset(new TSSLSocketFactory(TLSv1_0));
+ // Tell Thrift not to initialize SSL for us, as we use Kudu's SSL initializtion.
+ TSSLSocketFactory::setManualOpenSSLInitialization(true);
+ kudu::security::InitializeOpenSSL();
+ LOG(INFO) << "Initialized " << OPENSSL_VERSION_TEXT;
bool use_ldap = false;
diff --git a/be/src/rpc/authentication.h b/be/src/rpc/authentication.h
index 96d091b..f48fdcd 100644
--- a/be/src/rpc/authentication.h
+++ b/be/src/rpc/authentication.h
@@ -52,7 +52,7 @@ class AuthManager {
~AuthManager();
/// Set up internal and external AuthProvider classes. This also initializes SSL (via
- /// the creation of ssl_socket_factory_).
+ /// kudu::security::InitializeOpenSSL()).
Status Init();
/// Returns the authentication provider to use for "external" communication
@@ -81,14 +81,6 @@ class AuthManager {
boost::scoped_ptr<AuthProvider> internal_auth_provider_;
boost::scoped_ptr<AuthProvider> external_auth_provider_;
- /// A thrift SSL socket factory must be created and live the lifetime of the process to
- /// ensure that the thrift OpenSSL initialization code runs at Init(), and is not
- /// unregistered (which thrift will do when the refcount of TSSLSocketFactory objects
- /// reach 0), see IMPALA-4933. For simplicity, and because Kudu will expect SSL to be
- /// initialized, this will be created regardless of whether or not SSL credentials are
- /// specified. This factory isn't otherwise used.
- boost::scoped_ptr<TSSLSocketFactory> ssl_socket_factory_;
-
/// Used to authenticate usernames and passwords to LDAP.
std::unique_ptr<ImpalaLdap> ldap_;
};
diff --git a/be/src/rpc/thrift-server.cc b/be/src/rpc/thrift-server.cc
index abc3fcf..7d5a8c1 100644
--- a/be/src/rpc/thrift-server.cc
+++ b/be/src/rpc/thrift-server.cc
@@ -310,6 +310,11 @@ class ImpalaSslSocketFactory : public TSSLSocketFactory {
: TSSLSocketFactory(version), password_(password) {}
void ciphers(const string& enable) override {
+ if (ctx_.get() == nullptr) {
+ throw new TSSLException("ImpalaSslSocketFactory was not properly initialized.");
+ }
+ LOG(INFO) << "Enabling the following ciphers for the ImpalaSslSocketFactory: "
+ << enable;
SCOPED_OPENSSL_NO_PENDING_ERRORS;
TSSLSocketFactory::ciphers(enable);