You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2024/05/09 16:35:03 UTC
(impala) branch master updated (2a3ce2071 -> d1d28c0ee)
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from 2a3ce2071 IMPALA-12934: Added Calcite parsing files to Impala
new 85cd07a11 IMPALA-11499: Refactor UrlEncode function to handle special characters
new d1d28c0ee IMPALA-10451: Fix avro table loading failures caused by HIVE-24157
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/util/coding-util-test.cc | 14 ++++-
be/src/util/coding-util.cc | 39 ++++++++------
testdata/avro_schema_resolution/create_table.sql | 4 ++
.../functional-query/queries/QueryTest/insert.test | 18 +++++++
.../queries/QueryTest/unicode-column-name.test | 36 +++++++++++++
tests/query_test/test_insert.py | 63 ++++++++++++++++++++++
6 files changed, 155 insertions(+), 19 deletions(-)
(impala) 02/02: IMPALA-10451: Fix avro table loading failures caused by HIVE-24157
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit d1d28c0eec52c212b4b8cd09080327c542f24bfa
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Tue May 7 12:50:23 2024 -0700
IMPALA-10451: Fix avro table loading failures caused by HIVE-24157
HIVE-24157 introduces a restriction to prohibit casting DATE/TIMESTAMP
types to and from NUMERIC. It's enabled by default and can be turned off
by set hive.strict.timestamp.conversion=false.
This restriction breaks the data loading on avro_coldef and
avro_extra_coldef tables, which results in empty data set and finally
fails TestAvroSchemaResolution.test_avro_schema_resolution.
This patch explicitly disables the restriction in loading these two avro
tables.
The Hive version currently used for development does not have HIVE-24157,
but upstream Hive does have it. Adding hive.strict.timestamp.conversion
does not cause problems for Hive versions that don't have HIVE-24157.
Tests:
- Run the data loading and test_avro_schema_resolution locally using
a Hive that has HIVE-24157.
- Run CORE tests
- Run data loading with a Hive that doesn't have HIVE-24157.
Change-Id: I3e2a47d60d4079fece9c04091258215f3d6a7b52
Reviewed-on: http://gerrit.cloudera.org:8080/21413
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Joe McDonnell <jo...@cloudera.com>
---
testdata/avro_schema_resolution/create_table.sql | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/testdata/avro_schema_resolution/create_table.sql b/testdata/avro_schema_resolution/create_table.sql
index 27ae8e97c..01a31fdb7 100644
--- a/testdata/avro_schema_resolution/create_table.sql
+++ b/testdata/avro_schema_resolution/create_table.sql
@@ -166,6 +166,10 @@ TBLPROPERTIES ('avro.schema.literal'='{
-- any preexisting data files, which in turn will fail the query.
MSCK REPAIR TABLE avro_coldef;
+-- Disable the restriction of HIVE-24157, otherwise casting TIMESTAMP to BIGINT is
+-- prohibited. Note that type of timestamp_col is long in 'avro.schema.literal'.
+SET hive.strict.timestamp.conversion=false;
+
INSERT OVERWRITE TABLE avro_coldef PARTITION(year=2014, month=1)
SELECT bool_col, tinyint_col, smallint_col, int_col, bigint_col,
float_col, double_col, date_string_col, string_col, timestamp_col
(impala) 01/02: IMPALA-11499: Refactor UrlEncode function to handle special characters
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 85cd07a11e876f3d8773f2638f699c61a6b0dd4c
Author: pranavyl <pr...@cloudera.com>
AuthorDate: Fri Dec 8 19:11:28 2023 +0100
IMPALA-11499: Refactor UrlEncode function to handle special characters
An error came from an issue with URL encoding, where certain Unicode
characters were being incorrectly encoded due to their UTF-8
representation matching characters in the set of characters to escape.
For example, the string '运', which consists of three bytes
0xe8 0xbf 0x90 was wrongly getting encoded into '\E8%FFFFFFBF\90',
because the middle byte matched one of the two bytes that
represented the "\u00FF" literal. Inclusion of "\u00FF" was likely
a mistake from the beginning and it should have been '\x7F'.
The patch makes three key changes:
1. Before the change, the set of characters that need to be escaped
was stored as a string. The current patch uses an unordered_set
instead.
2. '\xFF', which is an invalid UTF-8 byte and whose inclusion was
erroneous from the beginning, is replaced with '\x7F', which is a
control character for DELETE, ensuring consistency and correctness in
URL encoding.
3. The list of characters to be escaped is extended to match the
current list in Hive.
Testing: Tests on both traditional Hive tables and Iceberg tables
are included in unicode-column-name.test, insert.test,
coding-util-test.cc and test_insert.py.
Change-Id: I88c4aba5d811dfcec809583d0c16fcbc0ca730fb
Reviewed-on: http://gerrit.cloudera.org:8080/21131
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/util/coding-util-test.cc | 14 ++++-
be/src/util/coding-util.cc | 39 ++++++++------
.../functional-query/queries/QueryTest/insert.test | 18 +++++++
.../queries/QueryTest/unicode-column-name.test | 36 +++++++++++++
tests/query_test/test_insert.py | 63 ++++++++++++++++++++++
5 files changed, 151 insertions(+), 19 deletions(-)
diff --git a/be/src/util/coding-util-test.cc b/be/src/util/coding-util-test.cc
index ad04222ae..7914de148 100644
--- a/be/src/util/coding-util-test.cc
+++ b/be/src/util/coding-util-test.cc
@@ -113,8 +113,18 @@ TEST(UrlCodingTest, BlankString) {
}
TEST(UrlCodingTest, PathSeparators) {
- TestUrl("/home/impala/directory/", "%2Fhome%2Fimpala%2Fdirectory%2F", false);
- TestUrl("/home/impala/directory/", "%2Fhome%2Fimpala%2Fdirectory%2F", true);
+ string test_path = "/home/impala/directory/";
+ string encoded_test_path = "%2Fhome%2Fimpala%2Fdirectory%2F";
+ TestUrl(test_path, encoded_test_path, false);
+ TestUrl(test_path, encoded_test_path, true);
+ string test = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r\x0E\x0F\x10"
+ "\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F\x7F\"#%'"
+ "*/:=?\\{[]^";
+ string encoded_test = "SpecialCharacters%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F"
+ "%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%7F%22%23%25"
+ "%27%2A%2F%3A%3D%3F%5C%7B%5B%5D%5E";
+ TestUrl(test, encoded_test, false);
+ TestUrl(test, encoded_test, true);
}
TEST(Base64Test, Basic) {
diff --git a/be/src/util/coding-util.cc b/be/src/util/coding-util.cc
index a78a3791d..7a7d1f6ae 100644
--- a/be/src/util/coding-util.cc
+++ b/be/src/util/coding-util.cc
@@ -18,16 +18,17 @@
#include "util/coding-util.h"
#include <cctype>
+#include <iomanip>
#include <limits>
#include <sstream>
+#include <unordered_set>
-#include <boost/algorithm/string/classification.hpp>
+#include <boost/algorithm/string.hpp>
#include <boost/function.hpp>
#include <sasl/sasl.h>
#include "common/compiler-util.h"
#include "common/logging.h"
-
#include "common/names.h"
#include "sasl/saslutil.h"
@@ -37,33 +38,37 @@ using std::uppercase;
namespace impala {
+// It is more convenient to maintain the complement of the set of
+// characters to escape when not in Hive-compat mode.
+static function<bool (char)> ShouldNotEscape = is_any_of("-_.~");
+
// Hive selectively encodes characters. This is the whitelist of
// characters it will encode.
// See common/src/java/org/apache/hadoop/hive/common/FileUtils.java
// in the Hive source code for the source of this list.
-static function<bool (char)> HiveShouldEscape = is_any_of("\"#%\\*/:=?\u00FF");
-
-// It is more convenient to maintain the complement of the set of
-// characters to escape when not in Hive-compat mode.
-static function<bool (char)> ShouldNotEscape = is_any_of("-_.~");
+static const std::unordered_set<char> SpecialCharacters = {
+ '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', '\b', '\t', '\n',
+ '\v', '\f', '\r', '\x0E', '\x0F', '\x10', '\x11', '\x12', '\x13', '\x14',
+ '\x15', '\x16', '\x17', '\x18', '\x19', '\x1A', '\x1B', '\x1C', '\x1D', '\x1E',
+ '\x1F', '\x7F', '"', '#', '%', '\'', '*', '/', ':', '=', '?', '\\', '{', '[', ']',
+ '^'};
static inline void UrlEncode(const char* in, int in_len, string* out, bool hive_compat) {
- (*out).reserve(in_len);
stringstream ss;
- for (int i = 0; i < in_len; ++i) {
- const char ch = in[i];
+ // "uppercase" and "hex" only affect the insertion of integers, not that of char values.
+ ss << uppercase << hex << setfill('0');
+ for (char ch : std::string_view(in, in_len)) {
// Escape the character iff a) we are in Hive-compat mode and the
- // character is in the Hive whitelist or b) we are not in
- // Hive-compat mode, and the character is not alphanumeric or one
- // of the four commonly excluded characters.
- if ((hive_compat && HiveShouldEscape(ch)) ||
- (!hive_compat && !(isalnum(ch) || ShouldNotEscape(ch)))) {
- ss << '%' << uppercase << hex << static_cast<uint32_t>(ch);
+ // character is in the Hive whitelist or b) we are not in Hive-compat mode and
+ // the character is not alphanumeric and it is not one of the characters specifically
+ // excluded from escaping (see ShouldNotEscape()).
+ if ((hive_compat && SpecialCharacters.count(ch) > 0) || (!hive_compat &&
+ !isalnum(static_cast<unsigned char>(ch)) && !ShouldNotEscape(ch))) {
+ ss << '%' << setw(2) << static_cast<uint32_t>(static_cast<unsigned char>(ch));
} else {
ss << ch;
}
}
-
(*out) = ss.str();
}
diff --git a/testdata/workloads/functional-query/queries/QueryTest/insert.test b/testdata/workloads/functional-query/queries/QueryTest/insert.test
index 88cec41bf..190fa1f36 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/insert.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/insert.test
@@ -446,6 +446,24 @@ SELECT * FROM insert_string_partitioned WHERE s2 = "/\%.";
string, string
====
---- QUERY
+truncate insert_string_partitioned;
+INSERT INTO TABLE insert_string_partitioned PARTITION(s2="O'Reilly") values ('0');
+SELECT * FROM insert_string_partitioned where s2= "O'Reilly";
+---- RESULTS
+'0','O''Reilly'
+---- TYPES
+STRING, STRING
+====
+---- QUERY
+truncate insert_string_partitioned;
+INSERT INTO TABLE insert_string_partitioned PARTITION(s2="Impala'#%*/:=?{}[]^") values ('0');
+SELECT * FROM insert_string_partitioned where s2= "Impala'#%*/:=?{}[]^";
+---- RESULTS
+'0','Impala''#%*/:=?{}[]^'
+---- TYPES
+STRING, STRING
+====
+---- QUERY
# static partition insert into string-partitioned table with non-escaped special characters
# (Hive chooses not to escape + and ' ')
truncate insert_string_partitioned;
diff --git a/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test b/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
index 59bd14dda..9d9f304da 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/unicode-column-name.test
@@ -291,3 +291,39 @@ INT, STRING
drop table testtbl_parquet;
---- RESULTS
'Table has been dropped.'
+====
+---- QUERY
+# Tests for IMPALA-11499
+create table unicode_partition_values (id int) partitioned by (p string) stored as parquet;
+---- RESULTS
+'Table has been created.'
+====
+---- QUERY
+insert into unicode_partition_values partition(p='运营业务数据') values (0);
+insert into unicode_partition_values partition(p='运') values (0);
+insert into unicode_partition_values partition(p='运营业务数据1234567890!@#$%^&*(){}[]') values (0);
+select * from unicode_partition_values;
+---- RESULTS: RAW_STRING
+0,'运'
+0,'运营业务数据'
+0,'运营业务数据1234567890!@#$%^&*(){}[]'
+---- TYPES
+INT, STRING
+====
+---- QUERY
+create table unicode_partition_values_iceberg (id int, p string) partitioned by spec (identity(p)) stored by iceberg;
+---- RESULTS
+'Table has been created.'
+====
+---- QUERY
+insert into unicode_partition_values_iceberg values (0, '运营业务数据');
+insert into unicode_partition_values_iceberg values (0, '运');
+insert into unicode_partition_values_iceberg values (0, '运营业务数据1234567890!@#$%^&*(){}[]');
+select * from unicode_partition_values_iceberg;
+---- RESULTS: RAW_STRING
+0,'运'
+0,'运营业务数据'
+0,'运营业务数据1234567890!@#$%^&*(){}[]'
+---- TYPES
+INT, STRING
+====
\ No newline at end of file
diff --git a/tests/query_test/test_insert.py b/tests/query_test/test_insert.py
index 0124ac932..81987a941 100644
--- a/tests/query_test/test_insert.py
+++ b/tests/query_test/test_insert.py
@@ -294,6 +294,69 @@ class TestInsertPartKey(ImpalaTestSuite):
self.run_test_case('QueryTest/insert_part_key', vector,
multiple_impalad=vector.get_value('exec_option')['sync_ddl'] == 1)
+ def test_escaped_partition_values(self, unique_database):
+ """Test for special characters in partition values."""
+ tbl = unique_database + ".tbl"
+ self.execute_query(
+ "create table {}(i int) partitioned by (p string) stored as parquet".format(tbl))
+ # "\\'" is used to represent a single quote since the insert statement uses a single
+ # quote on the partition value. "\\\\" is used for backslash since it gets escaped
+ # again in parsing the insert statement.
+ special_characters = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
+ "\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
+ "\x1C\x1D\x1E\x1F\"\x7F\\'%*/:=?\\\\{[]#^"
+ part_value = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
+ "\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
+ "\x1C\x1D\x1E\x1F\"\x7F'%*/:=?\\{[]#^"
+ part_dir = "p=SpecialCharacters%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F%" \
+ "10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%22%7F%27%25%2A" \
+ "%2F%3A%3D%3F%5C%7B%5B%5D%23%5E"
+ res = self.execute_query(
+ "insert into {} partition(p='{}') values (0)".format(tbl, special_characters))
+ assert res.data[0] == part_dir + ": 1"
+ res = self.client.execute("select p from {}".format(tbl))
+ assert res.data[0] == part_value
+ res = self.execute_query("show partitions " + tbl)
+ # There is a "\t" in the partition value making splitting of the result line
+ # difficult, hence we only verify that the value is present in string
+ # representation of the whole row.
+ assert part_value in res.data[0]
+ assert part_dir in res.data[0]
+
+ def test_escaped_partition_values_iceberg(self, unique_database):
+ """Test for special characters in partition values for iceberg tables"""
+ tbl = unique_database + ".tbl"
+ self.execute_query("create table {} (id int, p string) partitioned by"
+ " spec (identity(p)) stored by iceberg".format(tbl))
+ # "\\'" is used to represent a single quote since the insert statement uses a single
+ # quote on the partition value. "\\\\" is used for backslash since it gets escaped
+ # again in parsing the insert statement.
+ special_characters = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
+ "\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
+ "\x1C\x1D\x1E\x1F\"\x7F\\'%*/:=?\\\\{[]#^"
+ part_value = "SpecialCharacters\x01\x02\x03\x04\x05\x06\x07\b\t\n\v\f\r" \
+ "\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B" \
+ "\x1C\x1D\x1E\x1F\"\x7F'%*/:=?\\{[]#^"
+ part_dir = "p=SpecialCharacters%01%02%03%04%05%06%07%08%09%0A%0B%0C%0D%0E%0F%" \
+ "10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%22%7F%27%25%2A" \
+ "%2F%3A%3D%3F%5C%7B%5B%5D%23%5E"
+ show_part_value = "SpecialCharacters\\u0001\\u0002\\u0003\\u0004\\u0005\\u0006" \
+ "\\u0007\\b\\t\\n\\u000B\\f\\r\\u000E\\u000F\\u0010" \
+ "\\u0011\\u0012\\u0013\\u0014\\u0015\\u0016\\u0017" \
+ "\\u0018\\u0019\\u001A\\u001B\\u001C\\u001D\\u001E" \
+ "\\u001F\\\"\\u007F\'%*\\/:=?\\\\{[]#^"
+ res = self.execute_query(
+ "insert into {} values (0, '{}')".format(tbl, special_characters))
+ assert res.data[0] == part_dir + ": 1"
+ res = self.client.execute("select p from {}".format(tbl))
+ assert res.data[0] == part_value
+ res = self.execute_query("show partitions " + tbl)
+ # There is a "\t" in the partition value making splitting of the result line
+ # difficult, hence we only verify that the value is present in string
+ # representation of the whole row.
+ assert show_part_value in res.data[0]
+
+
class TestInsertNullQueries(ImpalaTestSuite):
@classmethod
def get_workload(self):