You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by bo...@apache.org on 2026/09/24 19:59:10 UTC
(impala) 01/02: IMPALA-12675: Set PARQUET_ANNOTATE_STRINGS_UTF8 to true by default and turn on validation
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 893783e1190f924fa5bb097029ae5a84477690c8
Author: Daniel Vanko <dv...@cloudera.com>
AuthorDate: Fri Jul 17 10:19:44 2026 +0200
IMPALA-12675: Set PARQUET_ANNOTATE_STRINGS_UTF8 to true by default and turn on validation
Now that Impala has a BINARY type for binary data and broader UTF-8
support, STRING columns written to Parquet are annotated with the
UTF-8 logical type by default (PARQUET_ANNOTATE_STRINGS_UTF8 now
defaults to true).
To keep the annotation truthful, STRING values are validated as UTF-8
on the Parquet write path (using the simdutf library) whenever the
annotation is enabled. A value that is not valid UTF-8 aborts the write
with PARQUET_INVALID_UTF8_STRING. Validation is an inseparable
consequence of the annotation: there is no option to keep the
annotation while writing unvalidated values, since that would produce a
file whose metadata lies about its contents. To store non-UTF-8 bytes,
set PARQUET_ANNOTATE_STRINGS_UTF8 to false to omit the annotation, or
use a BINARY column. CHAR/VARCHAR (always text) and BINARY columns are
not validated. Iceberg tables always annotate STRING columns as UTF-8
(a spec requirement that is not configurable), so their STRING values
are always validated; use a BINARY column to store non-UTF-8 bytes
there.
Details:
* Consume the simdutf 9.0.0 library from the native toolchain:
add IMPALA_SIMDUTF_VERSION, register the package in
bin/bootstrap_toolchain.py, locate it with a new
cmake_modules/FindSimdutf.cmake, and link it via IMPALA_DEPENDENCIES.
* Validate once per non-null value in
HdfsParquetTableWriter::AppendRow().
* Returning error if the string is not valid UTF-8, showing the
offending byte and its offset.
* Update the docs, including a new Impala 5.0.x entry in
impala_incompatible_changes.xml documenting the default flip as a
behaviour change.
Validation adds little to write time. CTAS of tpch.lineitem (6M rows)
to Parquet, 30 runs each on a local 3-node minicluster release build
(trimmed mean):
PARQUET_ANNOTATE_STRINGS_UTF8 | CTAS time
------------------------------+----------
false (no validation) | ~1.12s
true (validate) | ~1.22s
That is about +0.1s, or ~9%. The SIMD-accelerated simdutf scan is cheap
relative to the rest of the write path.
Testing:
* added new be test: Utf8UtilTest
* set PARQUET_ANNOTATE_STRINGS_UTF8=false in test_datasketches.py so it
can write the binary data into STRING columns (until IMPALA-9821)
* add new tests to test_iceberg.py and test_scanners.py
* modified existing e2e testcases to match new estimations
Change-Id: Ia95bcba978863ffb5d603fdf81bb2c75ba06f7da
Assisted-by: Claude Opus 4.8 (Claude Code)
Reviewed-on: http://gerrit.cloudera.org:8080/24570
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Zoltan Borok-Nagy <bo...@cloudera.com>
---
CMakeLists.txt | 5 +
be/CMakeLists.txt | 1 +
be/src/exec/parquet/hdfs-parquet-table-writer.cc | 32 +++++
be/src/exec/parquet/parquet-metadata-utils.cc | 5 +-
be/src/util/CMakeLists.txt | 3 +
be/src/util/utf8-util-test.cc | 97 ++++++++++++++++
be/src/util/utf8-util.cc | 40 +++++++
be/src/util/utf8-util.h | 35 ++++++
bin/bootstrap_toolchain.py | 4 +-
bin/impala-config.sh | 2 +
cmake_modules/FindSimdutf.cmake | 64 ++++++++++
common/thrift/Query.thrift | 10 +-
common/thrift/generate_error_codes.py | 9 +-
docs/topics/impala_incompatible_changes.xml | 21 ++++
.../impala_parquet_annotate_strings_utf8.xml | 27 +++--
.../org/apache/impala/planner/CardinalityTest.java | 6 +-
.../impala/calcite/planner/TestCalciteStats.java | 2 +-
.../queries/PlannerTest/pivot-clause.test | 20 ++--
.../queries/PlannerTest/unpivot-clause.test | 14 +--
.../queries/QueryTest/stats-extrapolation.test | 4 +-
tests/query_test/test_datasketches.py | 10 ++
tests/query_test/test_iceberg.py | 15 +++
tests/query_test/test_insert_parquet.py | 13 ++-
tests/query_test/test_parquet_page_index.py | 3 +
tests/query_test/test_scanners.py | 129 +++++++++++++++------
25 files changed, 495 insertions(+), 76 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5b8d1e066..8d1bff554 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -160,6 +160,7 @@ set_dep_root(CALLONCEHACK)
set_dep_root(CLOUDFLAREZLIB)
set_dep_root(OPENTELEMETRY_CPP)
set_dep_root(TCMALLOC)
+set_dep_root(SIMDUTF)
set(ABSL_ROOT
$ENV{IMPALA_TOOLCHAIN_PACKAGES_HOME}/abseil-cpp-$ENV{IMPALA_ABSEIL_CPP_VERSION})
@@ -349,6 +350,10 @@ IMPALA_ADD_THIRDPARTY_LIB(zstd ${ZSTD_INCLUDE_DIR} ${ZSTD_STATIC_LIB} "")
find_package(Re2 REQUIRED)
IMPALA_ADD_THIRDPARTY_LIB(re2 ${RE2_INCLUDE_DIR} ${RE2_STATIC_LIB} "")
+# find simdutf headers and libs
+find_package(Simdutf REQUIRED)
+IMPALA_ADD_THIRDPARTY_LIB(simdutf ${SIMDUTF_INCLUDE_DIR} ${SIMDUTF_STATIC_LIB} "")
+
# find jwt-cpp headers
find_package(JwtCpp REQUIRED)
include_directories(${JWT_CPP_INCLUDE_DIR})
diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index 0b47c145c..2fcad78b6 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -685,6 +685,7 @@ set (IMPALA_DEPENDENCIES
lz4
zstd
re2
+ simdutf
${Boost_LIBRARIES}
${LLVM_MODULE_LIBS}
thrift
diff --git a/be/src/exec/parquet/hdfs-parquet-table-writer.cc b/be/src/exec/parquet/hdfs-parquet-table-writer.cc
index ac610f891..e46b8b860 100644
--- a/be/src/exec/parquet/hdfs-parquet-table-writer.cc
+++ b/be/src/exec/parquet/hdfs-parquet-table-writer.cc
@@ -27,6 +27,7 @@
#include "exec/parquet/parquet-bloom-filter-util.h"
#include "exprs/scalar-expr-evaluator.h"
#include "exprs/scalar-expr.h"
+#include "gutil/stringprintf.h"
#include "rpc/thrift-util.h"
#include "runtime/date-value.h"
#include "runtime/decimal-value.h"
@@ -48,6 +49,7 @@
#include "util/pretty-printer.h"
#include "util/rle-encoding.h"
#include "util/string-util.h"
+#include "util/utf8-util.h"
#include <sstream>
#include <string>
@@ -144,6 +146,7 @@ class HdfsParquetTableWriter::BaseColumnWriter {
new RleEncoder(parent_->reusable_col_mem_pool_->Allocate(DEFAULT_DATA_PAGE_SIZE),
DEFAULT_DATA_PAGE_SIZE, 1));
values_buffer_ = parent_->reusable_col_mem_pool_->Allocate(values_buffer_len_);
+ validate_utf8_ = ShouldValidateUtf8();
}
virtual ~BaseColumnWriter() {}
@@ -338,6 +341,25 @@ class HdfsParquetTableWriter::BaseColumnWriter {
// Writes out the dictionary encoded data buffered in dict_encoder_.
void WriteDictDataPage();
+ // Whether values in this column must be validated as UTF-8: a non-binary STRING column
+ // written with the UTF-8 annotation. CHAR/VARCHAR are always annotated but are defined
+ // as text, and BINARY is never annotated as UTF-8 (see FillSchemaElement), so both are
+ // excluded.
+ bool ShouldValidateUtf8() const {
+ return parent_->string_utf8_ && type().type == TYPE_STRING && !type().IsBinaryType();
+ }
+
+ // Validates that 'value' (a non-null StringValue) is valid UTF-8, returning
+ // PARQUET_INVALID_UTF8_STRING (with the offset and hex of the offending byte) if not.
+ Status ValidateUtf8(const void* value) WARN_UNUSED_RESULT {
+ const StringValue* sv = reinterpret_cast<const StringValue*>(value);
+ int64_t pos = FindFirstInvalidUtf8(sv->Ptr(), sv->Len());
+ if (LIKELY(pos < 0)) return Status::OK();
+ uint8_t bad = static_cast<uint8_t>(sv->Ptr()[pos]);
+ return Status(TErrorCode::PARQUET_INVALID_UTF8_STRING, column_name(), pos,
+ StringPrintf("%02x", bad));
+ }
+
struct DataPage {
// Page header. This is a union of all page types.
parquet::PageHeader header;
@@ -441,6 +463,10 @@ class HdfsParquetTableWriter::BaseColumnWriter {
// True, if we should write the page index.
bool write_page_index_;
+ // Cached result of ShouldValidateUtf8(), set once in the ctor since the column type and
+ // annotation setting do not change over the writer's lifetime.
+ bool validate_utf8_ = false;
+
// Column descriptor from the HdfsTableDescriptor.
const ColumnDescriptor& col_desc_;
};
@@ -915,6 +941,12 @@ protected:
inline Status HdfsParquetTableWriter::BaseColumnWriter::AppendRow(TupleRow* row) {
++num_values_;
void* value = ConvertValue(expr_eval_->GetValue(row));
+ // Validate before encoding, so a rejected value fails before any partial
+ // state is committed. Runs once per non-null value here; ProcessValue() below may be
+ // retried on a new page.
+ if (validate_utf8_ && value != nullptr) {
+ RETURN_IF_ERROR(ValidateUtf8(value));
+ }
if (current_page_ == nullptr) NewPage();
int64_t bytes_needed = 0;
diff --git a/be/src/exec/parquet/parquet-metadata-utils.cc b/be/src/exec/parquet/parquet-metadata-utils.cc
index 6145e0f8a..99a544d3b 100644
--- a/be/src/exec/parquet/parquet-metadata-utils.cc
+++ b/be/src/exec/parquet/parquet-metadata-utils.cc
@@ -472,8 +472,9 @@ void ParquetMetadataUtils::FillSchemaElement(const ColumnType& col_type,
SetUtf8ConvertedAndLogicalType(col_schema);
break;
case TYPE_STRING:
- // By default STRING has no logical type, see IMPALA-5982.
- // VARCHAR and CHAR are always set to UTF8.
+ // Since Impala 5.0 (IMPALA-12675), STRING is annotated with the UTF-8 logical type
+ // by default (controlled by PARQUET_ANNOTATE_STRINGS_UTF8, on unless overridden).
+ // BINARY (a STRING subtype) is never annotated.
if (string_utf8 && !col_type.IsBinaryType()) {
SetUtf8ConvertedAndLogicalType(col_schema);
}
diff --git a/be/src/util/CMakeLists.txt b/be/src/util/CMakeLists.txt
index 0c8e4a86d..c41e33dd7 100644
--- a/be/src/util/CMakeLists.txt
+++ b/be/src/util/CMakeLists.txt
@@ -132,6 +132,7 @@ set(UTIL_SRCS
tuple-row-compare.cc
uid-util.cc
url-parser.cc
+ utf8-util.cc
variant-util.cc
version-util.cc
${SQUEASEL_SRC_DIR}/squeasel.c
@@ -316,6 +317,7 @@ add_library(UtilTests STATIC
tuple-row-compare-test.cc
uid-util-test.cc
uuid-util-test.cc
+ utf8-util-test.cc
variant-util-test.cc
version-util-test.cc
zip-util-test.cc
@@ -398,6 +400,7 @@ ADD_UNIFIED_BE_LSAN_TEST(time-test "TimeTest.*")
ADD_UNIFIED_BE_LSAN_TEST(tuple-row-compare-test "TupleRowCompareTest.*")
ADD_UNIFIED_BE_LSAN_TEST(uid-util-test "UidUtil.*")
ADD_UNIFIED_BE_LSAN_TEST(uuid-util-test "UuidUtil.*")
+ADD_UNIFIED_BE_LSAN_TEST(utf8-util-test "Utf8UtilTest.*")
ADD_UNIFIED_BE_LSAN_TEST(variant-util-test "VariantUtilTest.*")
ADD_UNIFIED_BE_LSAN_TEST(version-util-test "VersionUtilTest.*")
# Using standalone webserver-test for now, nonstandard main() passes in a port.
diff --git a/be/src/util/utf8-util-test.cc b/be/src/util/utf8-util-test.cc
new file mode 100644
index 000000000..f69b48522
--- /dev/null
+++ b/be/src/util/utf8-util-test.cc
@@ -0,0 +1,97 @@
+// 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.
+
+#include "testutil/gtest-util.h"
+
+#include "util/utf8-util.h"
+
+#include "common/names.h"
+
+namespace impala {
+
+// Helper so tests can pass string literals without computing lengths by hand.
+// Uses sizeof(literal) - 1 to exclude the trailing NUL, which also allows
+// embedded NUL bytes.
+#define EXPECT_VALID_UTF8(literal) \
+ EXPECT_TRUE(IsValidUtf8(literal, sizeof(literal) - 1))
+#define EXPECT_INVALID_UTF8(literal) \
+ EXPECT_FALSE(IsValidUtf8(literal, sizeof(literal) - 1))
+
+TEST(Utf8UtilTest, Empty) {
+ // An empty buffer is valid, and null is accepted when len == 0.
+ EXPECT_TRUE(IsValidUtf8(nullptr, 0));
+ EXPECT_TRUE(IsValidUtf8("", 0));
+}
+
+TEST(Utf8UtilTest, Ascii) {
+ EXPECT_VALID_UTF8("hello world");
+ // Embedded NUL is valid UTF-8.
+ EXPECT_TRUE(IsValidUtf8("a\0b", 3));
+}
+
+TEST(Utf8UtilTest, ValidMultibyte) {
+ EXPECT_VALID_UTF8("\xc3\xa9"); // U+00E9 é (2 bytes)
+ EXPECT_VALID_UTF8("\xd0\x91"); // U+0411 Б (2 bytes)
+ EXPECT_VALID_UTF8("\xe4\xbd\xa0\xe5\xa5\xbd"); // 你好 (3 bytes each)
+ EXPECT_VALID_UTF8("\xf0\x9f\x99\x82"); // U+1F642 🙂 (4 bytes)
+ EXPECT_VALID_UTF8("a\xc3\xa9\xe4\xbd\xa0\xf0\x9f\x99\x82z"); // mixed widths
+}
+
+TEST(Utf8UtilTest, LoneContinuationByte) {
+ EXPECT_INVALID_UTF8("\x80");
+ EXPECT_INVALID_UTF8("\xbf");
+ EXPECT_INVALID_UTF8("abc\x80xyz"); // invalid byte surrounded by valid ASCII
+}
+
+TEST(Utf8UtilTest, TruncatedSequence) {
+ EXPECT_INVALID_UTF8("\xc3"); // lead byte of a 2-byte seq, missing cont.
+ EXPECT_INVALID_UTF8("\xe4\xbd"); // 2 of 3 bytes
+ EXPECT_INVALID_UTF8("\xf0\x9f\x99"); // 3 of 4 bytes
+}
+
+TEST(Utf8UtilTest, InvalidContinuation) {
+ EXPECT_INVALID_UTF8("\xc3\x28"); // second byte is not a continuation byte
+ EXPECT_INVALID_UTF8("\xe4\x28\xa0");
+}
+
+TEST(Utf8UtilTest, InvalidBytes) {
+ EXPECT_INVALID_UTF8("\xff");
+ EXPECT_INVALID_UTF8("\xfe");
+ EXPECT_INVALID_UTF8("\xc0\x80"); // overlong encoding of U+0000
+}
+
+TEST(Utf8UtilTest, CodepointBoundaries) {
+ // U+10FFFF, the largest valid code point: F4 8F BF BF.
+ EXPECT_VALID_UTF8("\xf4\x8f\xbf\xbf");
+ // One past the maximum (U+110000): F4 90 80 80 is invalid.
+ EXPECT_INVALID_UTF8("\xf4\x90\x80\x80");
+ // UTF-16 surrogate half U+D800 (ED A0 80) is not valid UTF-8.
+ EXPECT_INVALID_UTF8("\xed\xa0\x80");
+}
+
+TEST(Utf8UtilTest, FindFirstInvalidPosition) {
+ // Valid inputs report -1.
+ EXPECT_EQ(-1, FindFirstInvalidUtf8(nullptr, 0));
+ EXPECT_EQ(-1, FindFirstInvalidUtf8("hello", 5));
+ EXPECT_EQ(-1, FindFirstInvalidUtf8("\xc3\xa9", 2)); // é
+ // The offset points at the first invalid byte.
+ EXPECT_EQ(0, FindFirstInvalidUtf8("\xff", 1));
+ EXPECT_EQ(3, FindFirstInvalidUtf8("abc\x80xyz", 7)); // lone continuation byte
+ EXPECT_EQ(2, FindFirstInvalidUtf8("ab\xc3", 3)); // truncated 2-byte sequence
+}
+
+} // namespace impala
diff --git a/be/src/util/utf8-util.cc b/be/src/util/utf8-util.cc
new file mode 100644
index 000000000..d793dc24d
--- /dev/null
+++ b/be/src/util/utf8-util.cc
@@ -0,0 +1,40 @@
+// 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.
+
+#include "util/utf8-util.h"
+
+#include <simdutf.h>
+
+namespace impala {
+
+bool IsValidUtf8(const char* ptr, size_t len) {
+ // An empty buffer is trivially valid UTF-8. Returning early also lets callers pass
+ // (nullptr, 0) without depending on simdutf's handling of a null pointer.
+ if (len == 0) return true;
+ return simdutf::validate_utf8(ptr, len);
+}
+
+int64_t FindFirstInvalidUtf8(const char* ptr, size_t len) {
+ if (len == 0) return -1;
+ simdutf::result result = simdutf::validate_utf8_with_errors(ptr, len);
+ // On error, 'count' is the byte offset of the error; on success it is the number of
+ // validated bytes, which we report as "valid" (-1).
+ if (result.error == simdutf::error_code::SUCCESS) return -1;
+ return static_cast<int64_t>(result.count);
+}
+
+} // namespace impala
diff --git a/be/src/util/utf8-util.h b/be/src/util/utf8-util.h
new file mode 100644
index 000000000..3ba9e3941
--- /dev/null
+++ b/be/src/util/utf8-util.h
@@ -0,0 +1,35 @@
+// 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.
+
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+
+namespace impala {
+
+/// Returns true if the buffer [ptr, ptr + len) contains a valid UTF-8 encoded
+/// string. An empty buffer (len == 0) is considered valid, in which case 'ptr'
+/// may be null. Backed by the SIMD-accelerated simdutf library.
+bool IsValidUtf8(const char* ptr, size_t len);
+
+/// Returns the byte offset of the first invalid UTF-8 sequence in [ptr, ptr + len), or
+/// -1 if the buffer is valid UTF-8 (an empty buffer is valid, in which case 'ptr' may be
+/// null). Backed by the SIMD-accelerated simdutf library.
+int64_t FindFirstInvalidUtf8(const char* ptr, size_t len);
+
+} // namespace impala
diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py
index 92fac50aa..494f66b52 100755
--- a/bin/bootstrap_toolchain.py
+++ b/bin/bootstrap_toolchain.py
@@ -475,8 +475,8 @@ def get_toolchain_downloads():
"calloncehack", "cctz", "cloudflarezlib", "cmake", "crcutil", "curl",
"flatbuffers", "gdb", "gflags", "glog", "gperftools", "jwt-cpp", "libev",
"libunwind", "lz4", "mold", "openldap", "opentelemetry-cpp", "orc",
- "protobuf", "python", "rapidjson", "re2", "snappy", "tcmalloc", "tpc-h",
- "tpc-ds", "zlib", "zstd"]]
+ "protobuf", "python", "rapidjson", "re2", "simdutf", "snappy", "tcmalloc",
+ "tpc-h", "tpc-ds", "zlib", "zstd"]]
gtest_package = ToolchainPackage(
"googletest", explicit_version=os.environ.get("IMPALA_GTEST_VERSION"))
toolchain_packages += [gtest_package]
diff --git a/bin/impala-config.sh b/bin/impala-config.sh
index 3625041e1..a6f55ddff 100755
--- a/bin/impala-config.sh
+++ b/bin/impala-config.sh
@@ -197,6 +197,8 @@ export IMPALA_RAPIDJSON_VERSION=1.1.0-p1
unset IMPALA_RAPIDJSON_URL
export IMPALA_RE2_VERSION=2023-03-01
unset IMPALA_RE2_URL
+export IMPALA_SIMDUTF_VERSION=9.0.0
+unset IMPALA_SIMDUTF_URL
export IMPALA_SNAPPY_VERSION=1.1.8
unset IMPALA_SNAPPY_URL
export IMPALA_SQUEASEL_VERSION=3.3
diff --git a/cmake_modules/FindSimdutf.cmake b/cmake_modules/FindSimdutf.cmake
new file mode 100644
index 000000000..480542659
--- /dev/null
+++ b/cmake_modules/FindSimdutf.cmake
@@ -0,0 +1,64 @@
+##############################################################################
+# 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.
+##############################################################################
+
+# - Find simdutf headers and lib.
+# SIMDUTF_ROOT hints the location
+# This module defines
+# SIMDUTF_INCLUDE_DIR, directory containing headers
+# SIMDUTF_STATIC_LIB, path to libsimdutf.a
+
+set(SIMDUTF_SEARCH_HEADER_PATHS ${SIMDUTF_ROOT}/include)
+
+set(SIMDUTF_SEARCH_LIB_PATHS ${SIMDUTF_ROOT}/lib)
+
+find_path(SIMDUTF_INCLUDE_DIR simdutf.h
+ PATHS ${SIMDUTF_SEARCH_HEADER_PATHS}
+ NO_DEFAULT_PATH
+ DOC "simdutf Unicode validation/transcoding header path"
+)
+
+find_library(SIMDUTF_LIBS NAMES simdutf
+ PATHS ${SIMDUTF_SEARCH_LIB_PATHS}
+ NO_DEFAULT_PATH
+ DOC "simdutf library"
+)
+
+find_library(SIMDUTF_STATIC_LIB NAMES libsimdutf.a
+ PATHS ${SIMDUTF_SEARCH_LIB_PATHS}
+ NO_DEFAULT_PATH
+ DOC "simdutf static library"
+)
+
+message(STATUS ${SIMDUTF_INCLUDE_DIR})
+
+if (NOT SIMDUTF_INCLUDE_DIR OR NOT SIMDUTF_LIBS OR
+ NOT SIMDUTF_STATIC_LIB)
+ set(SIMDUTF_FOUND FALSE)
+ message(FATAL_ERROR "simdutf includes and/or libraries NOT found. "
+ "Looked for headers in ${SIMDUTF_SEARCH_HEADER_PATHS}, "
+ "and for libs in ${SIMDUTF_SEARCH_LIB_PATHS}")
+else()
+ set(SIMDUTF_FOUND TRUE)
+endif ()
+
+mark_as_advanced(
+ SIMDUTF_INCLUDE_DIR
+ SIMDUTF_LIBS
+ SIMDUTF_STATIC_LIB
+)
diff --git a/common/thrift/Query.thrift b/common/thrift/Query.thrift
index 1bad0e6dd..6928215f0 100644
--- a/common/thrift/Query.thrift
+++ b/common/thrift/Query.thrift
@@ -267,12 +267,14 @@ struct TQueryOptions {
// Maximum number of bloom runtime filters allowed per query
41: optional i32 max_num_runtime_filters = 10
- // If true, use UTF-8 annotation for string columns. Note that char and varchar columns
+ // If true, use UTF-8 annotation for STRING columns. Note that CHAR and VARCHAR columns
// always use the annotation.
//
- // This is disabled by default in order to preserve the existing behavior of legacy
- // workloads. In addition, Impala strings are not necessarily UTF8-encoded.
- 42: optional bool parquet_annotate_strings_utf8 = false
+ // Enabled by default since Impala 5.0 (IMPALA-12675). A non-UTF-8 value aborts the
+ // write with PARQUET_INVALID_UTF8_STRING. Set to false to store non-UTF-8 bytes in
+ // STRING columns.
+ // Iceberg always annotates and validates STRING regardless of this option.
+ 42: optional bool parquet_annotate_strings_utf8 = true
// Determines how to resolve Parquet files' schemas in the absence of field IDs (which
// is always, since fields IDs are NYI). Valid values are "position" (default) and
diff --git a/common/thrift/generate_error_codes.py b/common/thrift/generate_error_codes.py
index c0bda655a..7c14452d3 100755
--- a/common/thrift/generate_error_codes.py
+++ b/common/thrift/generate_error_codes.py
@@ -507,7 +507,14 @@ error_codes = (
"catalogd $0 is in standby mode"),
("PARQUET_CORRUPT_DELTA_LENGTH_VALUE", 166, "File '$0' is corrupt: error decoding "
- "DELTA_LENGTH_BYTE_ARRAY value of type $1 at offset $2")
+ "DELTA_LENGTH_BYTE_ARRAY value of type $1 at offset $2"),
+
+ ("PARQUET_INVALID_UTF8_STRING", 167, "Column '$0' contains a value that is not valid "
+ "UTF-8 (invalid byte 0x$2 at offset $1), but the Parquet file is annotated as UTF-8 "
+ "(PARQUET_ANNOTATE_STRINGS_UTF8=true). To write such values, disable the "
+ "UTF-8 annotation (PARQUET_ANNOTATE_STRINGS_UTF8=false), or use a BINARY "
+ "column instead of STRING. Note that Iceberg tables always annotate STRING "
+ "columns as UTF-8, so a BINARY column is required there.")
)
import sys
diff --git a/docs/topics/impala_incompatible_changes.xml b/docs/topics/impala_incompatible_changes.xml
index 3c8f41f37..4c609378f 100644
--- a/docs/topics/impala_incompatible_changes.xml
+++ b/docs/topics/impala_incompatible_changes.xml
@@ -66,6 +66,27 @@ under the License.
fails with an analysis error.</p><p>To restore the previous behavior, set the
<codeph>EXPAND_COMPLEX_TYPES</codeph> query option to <codeph>false</codeph>.
See <xref href="impala_expand_complex_types.xml"/>.</p></li>
+ <li>STRING columns in Parquet are annotated as UTF-8 by default<p>Starting in
+ <keyword keyref="impala50"/>, the
+ <xref keyref="parquet_annotate_strings_utf8"/> defaults to
+ <codeph>true</codeph>, so Impala annotates <codeph>STRING</codeph> columns
+ written to Parquet with the UTF-8 logical type and validates that the values
+ are valid UTF-8. An <codeph>INSERT</codeph> or <codeph>CREATE TABLE AS
+ SELECT</codeph> that writes non-UTF-8 bytes (for example, binary or Latin-1
+ data) into a <codeph>STRING</codeph> Parquet column now fails with
+ <codeph>PARQUET_INVALID_UTF8_STRING</codeph>, where it previously
+ succeeded.</p><p>To store non-UTF-8 data, use a <codeph>BINARY</codeph>
+ column, or set <codeph>PARQUET_ANNOTATE_STRINGS_UTF8=false</codeph> to restore
+ the previous behavior. Iceberg tables always annotate <codeph>STRING</codeph>
+ columns as UTF-8 and cannot disable it, so a <codeph>BINARY</codeph> column is
+ required there.</p>
+ <note>Existing Iceberg tables that already contain non-UTF-8 bytes in
+ <codeph>STRING</codeph> columns may start to fail on operations that rewrite
+ data, such as <codeph>OPTIMIZE</codeph>, <codeph>UPDATE</codeph>, and
+ <codeph>MERGE</codeph>, because the rewritten values are validated on the
+ write path. To store such data, use a <codeph>BINARY</codeph> column
+ instead.</note>
+ </li>
</ul>
</p>
</conbody>
diff --git a/docs/topics/impala_parquet_annotate_strings_utf8.xml b/docs/topics/impala_parquet_annotate_strings_utf8.xml
index 0d4e09c63..190345faf 100644
--- a/docs/topics/impala_parquet_annotate_strings_utf8.xml
+++ b/docs/topics/impala_parquet_annotate_strings_utf8.xml
@@ -41,8 +41,11 @@ under the License.
</p>
<p conref="../shared/impala_common.xml#common/usage_notes_blurb"/>
- <p>
- By default, Impala represents a <codeph>STRING</codeph> column in Parquet as an unannotated binary field.
+ <p rev="5.0.0 IMPALA-12675">
+ Starting in <keyword keyref="impala50"/>, Impala annotates a <codeph>STRING</codeph> column in
+ Parquet with the UTF-8 logical type by default. In earlier releases, and when this option is set
+ to <codeph>false</codeph>, a <codeph>STRING</codeph> column is written as an unannotated binary
+ field.
</p>
<p>
Impala always uses the UTF-8 annotation when writing <codeph>CHAR</codeph> and <codeph>VARCHAR</codeph>
@@ -50,13 +53,23 @@ under the License.
values to <codeph>VARCHAR</codeph>.
</p>
<p>
- This option is to help make Impala-written data more interoperable with other data processing engines.
- Impala itself currently does not support all operations on UTF-8 data.
- Although data processed by Impala is typically represented in ASCII, it is valid to designate the
- data as UTF-8 when storing on disk, because ASCII is a subset of UTF-8.
+ The UTF-8 annotation helps make Impala-written data more interoperable with other data processing
+ engines. Impala itself currently does not support all operations on UTF-8 data.
+ Because ASCII is a subset of UTF-8, ASCII data is also valid UTF-8, so designating it as UTF-8 on
+ disk is safe.
+ </p>
+ <p rev="5.0.0 IMPALA-12675">
+ When the UTF-8 annotation is enabled, Impala validates that <codeph>STRING</codeph> values written
+ to Parquet are valid UTF-8, so that the annotation is truthful. A value that is not valid UTF-8
+ causes the write to fail. To store non-UTF-8 data (for example, binary or Latin-1 bytes), set this
+ option to <codeph>false</codeph> to omit the annotation, or use a <codeph>BINARY</codeph> column
+ instead of <codeph>STRING</codeph>. <codeph>CHAR</codeph> and <codeph>VARCHAR</codeph> columns,
+ which are always annotated as UTF-8, are not validated. Iceberg tables always annotate
+ <codeph>STRING</codeph> columns as UTF-8 regardless of this option, so a <codeph>BINARY</codeph>
+ column is required to store non-UTF-8 bytes there.
</p>
<p conref="../shared/impala_common.xml#common/type_boolean"/>
- <p conref="../shared/impala_common.xml#common/default_false_0"/>
+ <p conref="../shared/impala_common.xml#common/default_true_1"/>
<p conref="../shared/impala_common.xml#common/added_in_260"/>
diff --git a/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java b/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
index 8b760b39c..4489a1ed5 100644
--- a/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
+++ b/fe/src/test/java/org/apache/impala/planner/CardinalityTest.java
@@ -584,11 +584,11 @@ public class CardinalityTest extends PlannerTestBase {
// There is no available statistics in functional_parquet.alltypestiny.
// True cardinality of functional_parquet.alltypestiny is 8.
- // Estimated cardinality of functional_parquet.alltypestiny is 742.
- // Estimated cardinality of the NestedLoopJoinNode is 550,564 = 742 * 742.
+ // Estimated cardinality of functional_parquet.alltypestiny is 761.
+ // Estimated cardinality of the NestedLoopJoinNode is 579,121 = 761 * 761.
verifyApproxCardinality("SELECT * "
+ "FROM functional_parquet.alltypestiny a, "
- + "functional_parquet.alltypestiny b", 550_564, true,
+ + "functional_parquet.alltypestiny b", 579_121, true,
ImmutableSet.of(), path, NestedLoopJoinNode.class);
}
diff --git a/java/calcite-planner/src/test/java/org/apache/impala/calcite/planner/TestCalciteStats.java b/java/calcite-planner/src/test/java/org/apache/impala/calcite/planner/TestCalciteStats.java
index fc0e6bbf1..57837e723 100644
--- a/java/calcite-planner/src/test/java/org/apache/impala/calcite/planner/TestCalciteStats.java
+++ b/java/calcite-planner/src/test/java/org/apache/impala/calcite/planner/TestCalciteStats.java
@@ -738,7 +738,7 @@ public class TestCalciteStats extends PlannerTestBase {
assertEquals(1.0, (double) mq.getRowCount(getRelNodeForQuery("SELECT" +
" bigint_col, id FROM functional_parquet.alltypes where bigint_col = 10")),
DOUBLE_ERR);
- assertEquals(758.0, (double) mq.getRowCount(getRelNodeForQuery(
+ assertEquals(761.0, (double) mq.getRowCount(getRelNodeForQuery(
"SELECT a.id FROM functional_parquet.alltypes a inner join " +
"functional_parquet.alltypestiny b on (a.id= b.id)")), DOUBLE_ERR);
} catch (ImpalaException e) {
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/pivot-clause.test b/testdata/workloads/functional-planner/queries/PlannerTest/pivot-clause.test
index 2b4721aed..6b4fe7327 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/pivot-clause.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/pivot-clause.test
@@ -35,7 +35,7 @@ PLAN-ROOT SINK
|
00:SCAN HDFS [functional_parquet.alltypestiny]
partition predicates: functional_parquet.alltypestiny.year = 2009
- HDFS partitions=4/4 files=4 size=11.92KB
+ HDFS partitions=4/4 files=4 size=11.96KB
partition key scan
row-size=8B cardinality=4
====
@@ -76,7 +76,7 @@ PLAN-ROOT SINK
| row-size=12B cardinality=4
|
00:SCAN HDFS [functional_parquet.alltypestiny]
- HDFS partitions=4/4 files=4 size=11.92KB
+ HDFS partitions=4/4 files=4 size=11.96KB
partition key scan
row-size=8B cardinality=4
====
@@ -96,7 +96,7 @@ PLAN-ROOT SINK
04:HASH JOIN [INNER JOIN, BROADCAST]
| hash predicates: t2.`year` = `year`
| runtime filters: RF000 <- `year`, RF001 <- `year`
-| row-size=16B cardinality=758
+| row-size=16B cardinality=761
|
|--09:EXCHANGE [BROADCAST]
| |
@@ -125,14 +125,14 @@ PLAN-ROOT SINK
| | row-size=12B cardinality=4
| |
| 00:SCAN HDFS [functional_parquet.alltypestiny]
-| HDFS partitions=4/4 files=4 size=11.92KB
+| HDFS partitions=4/4 files=4 size=11.96KB
| partition key scan
| row-size=8B cardinality=4
|
03:SCAN HDFS [functional_parquet.alltypestiny t2]
- HDFS partitions=4/4 files=4 size=11.92KB
+ HDFS partitions=4/4 files=4 size=11.96KB
runtime filters: RF001 -> t2.`year`, RF000 -> t2.`year`
- row-size=4B cardinality=758
+ row-size=4B cardinality=761
====
# The output of the AGGREGATE node is consumed by a JOIN node on a column
# created from the split.
@@ -151,7 +151,7 @@ PLAN-ROOT SINK
04:HASH JOIN [INNER JOIN, BROADCAST]
| hash predicates: t2.`month` = aggif(`month` = 2, result)
| runtime filters: RF000 <- aggif(`month` = 2, result), RF001 <- aggif(`month` = 2, result)
-| row-size=16B cardinality=758
+| row-size=16B cardinality=761
|
|--09:EXCHANGE [BROADCAST]
| |
@@ -180,14 +180,14 @@ PLAN-ROOT SINK
| | row-size=12B cardinality=4
| |
| 00:SCAN HDFS [functional_parquet.alltypestiny]
-| HDFS partitions=4/4 files=4 size=11.92KB
+| HDFS partitions=4/4 files=4 size=11.96KB
| partition key scan
| row-size=8B cardinality=4
|
03:SCAN HDFS [functional_parquet.alltypestiny t2]
- HDFS partitions=4/4 files=4 size=11.92KB
+ HDFS partitions=4/4 files=4 size=11.96KB
runtime filters: RF001 -> t2.`month`, RF000 -> t2.`month`
- row-size=4B cardinality=758
+ row-size=4B cardinality=761
====
# Aggregates with multiple DISTINCTs in the PIVOT clause.
with t1 (a, b, c) as (
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/unpivot-clause.test b/testdata/workloads/functional-planner/queries/PlannerTest/unpivot-clause.test
index 1cf504be7..6a7f46837 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/unpivot-clause.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/unpivot-clause.test
@@ -16,7 +16,7 @@ PLAN-ROOT SINK
|
00:SCAN HDFS [functional_parquet.alltypestiny]
partition predicates: functional_parquet.alltypestiny.month = 1
- HDFS partitions=1/4 files=1 size=2.98KB
+ HDFS partitions=1/4 files=1 size=2.99KB
row-size=12B cardinality=190
====
# The child node of the UNPIVOT node is a SCAN node.
@@ -37,8 +37,8 @@ PLAN-ROOT SINK
| row-size=20B cardinality=1.52K
|
00:SCAN HDFS [functional_parquet.alltypestiny]
- HDFS partitions=4/4 files=4 size=11.92KB
- row-size=12B cardinality=758
+ HDFS partitions=4/4 files=4 size=11.96KB
+ row-size=12B cardinality=761
====
# The child node of the UNPIVOT node is a JOIN node.
select t1.id, b, c, t2.id from functional_parquet.alltypestiny unpivot (
@@ -57,8 +57,8 @@ PLAN-ROOT SINK
|--04:EXCHANGE [BROADCAST]
| |
| 02:SCAN HDFS [functional_parquet.alltypestiny t2]
-| HDFS partitions=4/4 files=4 size=11.92KB
-| row-size=8B cardinality=758
+| HDFS partitions=4/4 files=4 size=11.96KB
+| row-size=8B cardinality=761
|
01:UNPIVOT
| source-exprs=functional_parquet.alltypestiny.id
@@ -66,6 +66,6 @@ PLAN-ROOT SINK
| row-size=20B cardinality=1.52K
|
00:SCAN HDFS [functional_parquet.alltypestiny]
- HDFS partitions=4/4 files=4 size=11.92KB
- row-size=12B cardinality=758
+ HDFS partitions=4/4 files=4 size=11.96KB
+ row-size=12B cardinality=761
====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test b/testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
index 5e3424228..a40d1a478 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
@@ -11,10 +11,10 @@ explain select id from alltypes;
---- RESULTS: VERIFY_IS_SUBSET
' stored statistics:'
' table: rows=unavailable size=unavailable'
-' partitions: 0/12 rows=5.97K'
+' partitions: 0/12 rows=5.98K'
' columns: unavailable'
row_regex:.* extrapolated-rows=unavailable.*
-' tuple-ids=0 row-size=4B cardinality=5.97K'
+' tuple-ids=0 row-size=4B cardinality=5.98K'
---- TYPES
STRING
====
diff --git a/tests/query_test/test_datasketches.py b/tests/query_test/test_datasketches.py
index 1be7b5dc1..943aee4ba 100644
--- a/tests/query_test/test_datasketches.py
+++ b/tests/query_test/test_datasketches.py
@@ -31,19 +31,29 @@ class TestDatasketches(ImpalaTestSuite):
def test_hll(self, vector, unique_database):
create_table_from_parquet(self.client, unique_database, 'hll_sketches_from_hive')
create_table_from_parquet(self.client, unique_database, 'hll_sketches_from_impala')
+ # Sketches are serialized binary data stored in STRING columns, so they are not valid
+ # UTF-8. Disable the annotation until IMPALA-9821 makes these functions return BINARY,
+ # after which setting the option will no longer be needed.
+ vector.get_value('exec_option')['parquet_annotate_strings_utf8'] = False
self.run_test_case('QueryTest/datasketches-hll', vector, unique_database)
def test_cpc(self, vector, unique_database):
create_table_from_parquet(self.client, unique_database, 'cpc_sketches_from_hive')
create_table_from_parquet(self.client, unique_database, 'cpc_sketches_from_impala')
+ # Binary sketches aren't UTF-8; disable the annotation until IMPALA-9821.
+ vector.get_value('exec_option')['parquet_annotate_strings_utf8'] = False
self.run_test_case('QueryTest/datasketches-cpc', vector, unique_database)
def test_theta(self, vector, unique_database):
create_table_from_parquet(self.client, unique_database, 'theta_sketches_from_hive')
create_table_from_parquet(self.client, unique_database, 'theta_sketches_from_impala')
+ # Binary sketches aren't UTF-8; disable the annotation until IMPALA-9821.
+ vector.get_value('exec_option')['parquet_annotate_strings_utf8'] = False
self.run_test_case('QueryTest/datasketches-theta', vector, unique_database)
def test_kll(self, vector, unique_database):
create_table_from_parquet(self.client, unique_database, 'kll_sketches_from_hive')
create_table_from_parquet(self.client, unique_database, 'kll_sketches_from_impala')
+ # Binary sketches aren't UTF-8; disable the annotation until IMPALA-9821.
+ vector.get_value('exec_option')['parquet_annotate_strings_utf8'] = False
self.run_test_case('QueryTest/datasketches-kll', vector, unique_database)
diff --git a/tests/query_test/test_iceberg.py b/tests/query_test/test_iceberg.py
index 9f55c2809..15e3b1c50 100644
--- a/tests/query_test/test_iceberg.py
+++ b/tests/query_test/test_iceberg.py
@@ -96,6 +96,21 @@ class TestIcebergTable(IcebergTestSuite):
table_location="${IMPALA_HOME}/testdata/data/iceberg_test/iceberg_uuid")
self.run_test_case('QueryTest/iceberg-uuid-type', vector, use_db=unique_database)
+ def test_utf8_string_validation(self, unique_database):
+ """IMPALA-12675: Iceberg always annotates STRING as UTF-8, so writing non-UTF-8 bytes
+ to a STRING column always fails; a BINARY column stores them instead."""
+ str_tbl = unique_database + ".utf8_string"
+ bin_tbl = unique_database + ".utf8_binary"
+ self.client.execute("create table {0} (s string) stored as iceberg".format(str_tbl))
+ self.client.execute("create table {0} (b binary) stored as iceberg".format(bin_tbl))
+ # UNHEX('ff') is a single 0xFF byte, which is not valid UTF-8.
+ err = self.execute_query_expect_failure(
+ self.client, "insert into {0} values (unhex('ff'))".format(str_tbl))
+ assert "not valid UTF-8" in str(err)
+ # The same bytes are valid in a BINARY column (the recommended alternative).
+ bin_insert = "insert into {0} values (cast(unhex('ff') as binary))".format(bin_tbl)
+ self.execute_query_expect_success(self.client, bin_insert)
+
def test_external_iceberg_tables(self, vector, unique_database):
self.run_test_case('QueryTest/iceberg-external', vector, unique_database)
diff --git a/tests/query_test/test_insert_parquet.py b/tests/query_test/test_insert_parquet.py
index 70a918fa3..56723cf33 100644
--- a/tests/query_test/test_insert_parquet.py
+++ b/tests/query_test/test_insert_parquet.py
@@ -431,6 +431,15 @@ class TestHdfsParquetTableWriter(ImpalaTestSuite):
assert schema.converted_type is None
assert schema.logicalType is None
+ def _check_utf8_logical_type(self, schemas, column_name):
+ """Checks that the schema with name 'column_name' is annotated as a UTF-8 string,
+ i.e. it has the UTF8 converted type and the STRING logical type."""
+ schema = self._get_schema(schemas, column_name)
+ assert schema.converted_type == ConvertedType.UTF8
+ assert schema.logicalType is not None
+ self._check_only_one_member_var_is_set(schema.logicalType, "STRING")
+ assert schema.logicalType.STRING is not None
+
def _check_int_logical_type(self, schemas, column_name, bit_width):
"""Checks that the schema with name 'column_name' has logical and converted type that
describe a signed integer with 'bit_width' bits."""
@@ -477,8 +486,8 @@ class TestHdfsParquetTableWriter(ImpalaTestSuite):
self._check_no_logical_type(schemas, "float_col")
self._check_no_logical_type(schemas, "double_col")
- # By default STRING has no logical type, see IMPALA-5982.
- self._check_no_logical_type(schemas, "string_col")
+ # STRING is annotated with the UTF-8 logical type by default.
+ self._check_utf8_logical_type(schemas, "string_col")
# Currently TIMESTAMP is written as INT96 and has no logical type.
# This test will break once INT64 becomes the default Parquet type for TIMESTAMP
diff --git a/tests/query_test/test_parquet_page_index.py b/tests/query_test/test_parquet_page_index.py
index 351e7e203..4132ecd90 100644
--- a/tests/query_test/test_parquet_page_index.py
+++ b/tests/query_test/test_parquet_page_index.py
@@ -351,6 +351,9 @@ class TestHdfsParquetTableIndexWriter(ImpalaTestSuite):
"""Test string values that are all 0xFFs or end with 0xFFs."""
col_type = "STRING"
+ # These values are deliberately not valid UTF-8, so disable the annotation on the
+ # Parquet write path to skip validation as well.
+ vector.get_value('exec_option')['parquet_annotate_strings_utf8'] = False
# String value is all of 0xFFs but its length is less than PAGE_INDEX_TRUNCATE_LENGTH.
short_tbl = "short_tbl"
short_hdfs_path = self._create_table_with_values_of_type(col_type, vector,
diff --git a/tests/query_test/test_scanners.py b/tests/query_test/test_scanners.py
index 65c75e5a5..f5e5a0743 100644
--- a/tests/query_test/test_scanners.py
+++ b/tests/query_test/test_scanners.py
@@ -53,7 +53,7 @@ from tests.common.test_dimensions import (
from tests.common.test_result_verifier import parse_result_rows, QueryTestResult
from tests.common.test_vector import ImpalaTestDimension
from tests.util.filesystem_utils import get_fs_path, IS_HDFS
-from tests.util.get_parquet_metadata import get_parquet_metadata
+from tests.util.get_parquet_metadata import get_parquet_metadata_from_hdfs_folder
from tests.util.parse_util import get_bytes_summary_stats_counter
from tests.util.test_file_parser import QueryTestSectionReader
@@ -927,7 +927,17 @@ class TestParquet(ImpalaTestSuite):
# Assert that (min == avg == max)
assert min_max_time[0] == min_max_time[1] == min_max_time[2] != 0
- def test_annotate_utf8_option(self, vector, unique_database):
+ def _get_parquet_schema_columns(self, tmpdir, unique_database, table_name):
+ """Returns {column_name: SchemaElement} for the single Parquet file of
+ `unique_database.table_name`. Uses a fresh subdirectory of `tmpdir` so it can be
+ called again after the table is overwritten."""
+ hdfs_path = get_fs_path('/test-warehouse/%s.db/%s' % (unique_database, table_name))
+ dest_dir = tempfile.mkdtemp(dir=tmpdir.strpath)
+ file_metadata_list = get_parquet_metadata_from_hdfs_folder(hdfs_path, dest_dir)
+ assert len(file_metadata_list) == 1
+ return {element.name: element for element in file_metadata_list[0].schema}
+
+ def test_annotate_utf8_option(self, vector, unique_database, tmpdir):
if self.exploration_strategy() != 'exhaustive': pytest.skip("Only run in exhaustive")
# Create table
@@ -938,41 +948,19 @@ class TestParquet(ImpalaTestSuite):
'stored as parquet' % qualified_table_name
self.execute_query(query, options)
- # Insert data that should have UTF8 annotation
+ # Insert data that should have UTF8 annotation.
query = 'insert overwrite table %s '\
'values("a", cast("b" as char(10)), cast("c" as varchar(10)), "d")' \
% qualified_table_name
options['parquet_annotate_strings_utf8'] = True
self.execute_query(query, options)
- def get_schema_elements():
- # Copy the created file to the local filesystem and parse metadata
- local_file = '/tmp/utf8_test_%s.parq' % random.randint(0, 10000)
- LOG.info("test_annotate_utf8_option local file name: " + local_file)
- hdfs_file = get_fs_path('/test-warehouse/%s.db/%s/*.parq'
- % (unique_database, TABLE_NAME))
- check_call(['hadoop', 'fs', '-copyToLocal', hdfs_file, local_file])
- metadata = get_parquet_metadata(local_file)
-
- # Extract SchemaElements corresponding to the table columns
- a_schema_element = metadata.schema[1]
- assert a_schema_element.name == 'a'
- b_schema_element = metadata.schema[2]
- assert b_schema_element.name == 'b'
- c_schema_element = metadata.schema[3]
- assert c_schema_element.name == 'c'
- d_schema_element = metadata.schema[4]
- assert d_schema_element.name == 'd'
-
- os.remove(local_file)
- return a_schema_element, b_schema_element, c_schema_element, d_schema_element
-
# Check that the schema uses the UTF8 annotation
- a_schema_elt, b_schema_elt, c_schema_elt, d_schema_elt = get_schema_elements()
- assert a_schema_elt.converted_type == ConvertedType.UTF8
- assert b_schema_elt.converted_type == ConvertedType.UTF8
- assert c_schema_elt.converted_type == ConvertedType.UTF8
- assert d_schema_elt.converted_type == ConvertedType.UTF8
+ cols = self._get_parquet_schema_columns(tmpdir, unique_database, TABLE_NAME)
+ assert cols['a'].converted_type == ConvertedType.UTF8
+ assert cols['b'].converted_type == ConvertedType.UTF8
+ assert cols['c'].converted_type == ConvertedType.UTF8
+ assert cols['d'].converted_type == ConvertedType.UTF8
# Create table and insert data that should not have UTF8 annotation for strings
options['parquet_annotate_strings_utf8'] = False
@@ -980,11 +968,82 @@ class TestParquet(ImpalaTestSuite):
# Check that the schema does not use the UTF8 annotation except for CHAR and VARCHAR
# columns
- a_schema_elt, b_schema_elt, c_schema_elt, d_schema_elt = get_schema_elements()
- assert a_schema_elt.converted_type is None
- assert b_schema_elt.converted_type == ConvertedType.UTF8
- assert c_schema_elt.converted_type == ConvertedType.UTF8
- assert d_schema_elt.converted_type is None
+ cols = self._get_parquet_schema_columns(tmpdir, unique_database, TABLE_NAME)
+ assert cols['a'].converted_type is None
+ assert cols['b'].converted_type == ConvertedType.UTF8
+ assert cols['c'].converted_type == ConvertedType.UTF8
+ assert cols['d'].converted_type is None
+
+ def test_utf8_string_validation(self, vector, unique_database, tmpdir):
+ """UTF-8 annotation is enabled by default and this means that validation too, so
+ writing a STRING value that is not valid UTF-8 to Parquet fails. The only way to write
+ such a value into a STRING column is to disable the annotation
+ (PARQUET_ANNOTATE_STRINGS_UTF8=false)."""
+ table_name = "utf8_validation"
+ qualified_table_name = "%s.%s" % (unique_database, table_name)
+ self.execute_query(
+ "create table %s (s string) stored as parquet" % qualified_table_name)
+
+ # unhex('ff') produces a single 0xFF byte, which is not valid UTF-8.
+ invalid = "insert overwrite %s values (unhex('ff'))" % qualified_table_name
+
+ def s_converted_type():
+ cols = self._get_parquet_schema_columns(tmpdir, unique_database, table_name)
+ return cols['s'].converted_type
+
+ # Valid UTF-8 with the default options (annotation on, validation on).
+ self.execute_query("insert overwrite %s values ('hello')" % qualified_table_name,
+ vector.get_value('exec_option'))
+ assert s_converted_type() == ConvertedType.UTF8
+
+ # An invalid UTF-8 value is rejected by default.
+ err = self.execute_query_expect_failure(self.client, invalid,
+ vector.get_value('exec_option'))
+ assert "not valid UTF-8" in str(err)
+
+ # The failed INSERT OVERWRITE must not have altered the table.
+ result = self.execute_query("select s from %s" % qualified_table_name,
+ vector.get_value('exec_option'))
+ assert result.data == ['hello']
+
+ # Disabling the annotation
+ opts = deepcopy(vector.get_value('exec_option'))
+ opts['parquet_annotate_strings_utf8'] = False
+ self.execute_query(invalid, opts)
+ assert s_converted_type() is None
+
+ src_tbl = "%s.utf8_validation_src" % unique_database
+ self.execute_query("create table %s (s string) stored as parquet" % src_tbl)
+ self.execute_query("insert into %s values (unhex('ff'))" % src_tbl, opts)
+
+ err = self.execute_query_expect_failure(self.client,
+ "insert overwrite %s select s from %s" % (qualified_table_name, src_tbl),
+ vector.get_value('exec_option'))
+ assert "not valid UTF-8" in str(err)
+
+ ctas_tbl = "%s.utf8_validation_ctas" % unique_database
+ err = self.execute_query_expect_failure(self.client,
+ "create table %s as select s from %s" % (ctas_tbl, src_tbl),
+ vector.get_value('exec_option'))
+ assert "not valid UTF-8" in str(err)
+
+ def test_utf8_varchar_not_validated(self, vector, unique_database, tmpdir):
+ """CHAR/VARCHAR columns are always annotated as UTF-8 but, unlike STRING, are not
+ validated. Writing non-UTF-8 bytes into a VARCHAR column therefore succeeds with the
+ default options while the UTF-8 annotation is still emitted -- it's a documented
+ asymmetry vs. STRING (see impala_parquet_annotate_strings_utf8.xml)."""
+ table_name = "utf8_varchar"
+ qualified_table_name = "%s.%s" % (unique_database, table_name)
+ self.execute_query(
+ "create table %s (c varchar(10)) stored as parquet" % qualified_table_name)
+ self.execute_query(
+ "insert overwrite %s values (cast(unhex('ff') as varchar(10)))"
+ % qualified_table_name,
+ vector.get_value('exec_option'))
+
+ # The VARCHAR column is still annotated as UTF-8.
+ cols = self._get_parquet_schema_columns(tmpdir, unique_database, table_name)
+ assert cols['c'].converted_type == ConvertedType.UTF8
def test_resolution_by_name(self, vector, unique_database):
self.run_test_case('QueryTest/parquet-resolution-by-name', vector,