You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by cs...@apache.org on 2026/09/29 11:52:49 UTC

(impala) branch master updated (5c42a6cdb -> 1a23e7759)

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


    from 5c42a6cdb IMPALA-15360: Fix /catalog estimated memory column fails to render
     new 66db905e0 IMPALA-15422: Fix fall-through in system table DECIMAL writer
     new 1a23e7759 IMPALA-15411: Add is_valid_utf8() builtin

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/exec/system-table-scanner.cc          |  5 ++++
 be/src/exprs/expr-test.cc                    | 11 +++++++++
 be/src/exprs/string-functions-ir.cc          |  8 +++++++
 be/src/exprs/string-functions.h              |  2 ++
 common/function-registry/impala_functions.py |  1 +
 common/thrift/generate_error_codes.py        |  3 ++-
 docs/topics/impala_incompatible_changes.xml  |  5 +++-
 docs/topics/impala_string_functions.xml      | 36 ++++++++++++++++++++++++++++
 tests/custom_cluster/test_query_live.py      | 24 +++++++++++++++++++
 tests/query_test/test_scanners.py            |  8 +++++++
 10 files changed, 101 insertions(+), 2 deletions(-)


(impala) 02/02: IMPALA-15411: Add is_valid_utf8() builtin

Posted by cs...@apache.org.
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 1a23e7759d1c9cb6dfc9709c50d578b3d1111bf4
Author: Daniel Vanko <dv...@cloudera.com>
AuthorDate: Fri Sep 25 19:29:22 2026 +0200

    IMPALA-15411: Add is_valid_utf8() builtin
    
    IMPALA-12675 turned on UTF-8 validation on the Parquet write path, so an
    INSERT or CTAS writing non-UTF-8 bytes into an annotated STRING column
    fails with PARQUET_INVALID_UTF8_STRING. The error points at one byte,
    but there was no way to find which rows are affected.
    
    is_valid_utf8(STRING) -> BOOLEAN closes that gap:
    
      SELECT * FROM t WHERE NOT is_valid_utf8(s);
    
    It calls impala::IsValidUtf8(), the same simdutf-based primitive the
    Parquet writer uses, so it accepts exactly what the write path accepts.
    It validates bytes, not characters, so the result does not depend on
    UTF8_MODE. NULL returns NULL, the empty string is valid.
    
    The error message and the incompatible-changes note now name it.
    
    Testing:
     - extended Utf8Test
     - added is_valid_utf8() call and error message check to e2e test
    
    Change-Id: Ia1c8b537bdfc7c35839d53eb43fbe841c71c4a51
    Assisted-by: Claude Opus 5 (Claude Code)
    Reviewed-on: http://gerrit.cloudera.org:8080/24950
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exprs/expr-test.cc                    | 11 +++++++++
 be/src/exprs/string-functions-ir.cc          |  8 +++++++
 be/src/exprs/string-functions.h              |  2 ++
 common/function-registry/impala_functions.py |  1 +
 common/thrift/generate_error_codes.py        |  3 ++-
 docs/topics/impala_incompatible_changes.xml  |  5 +++-
 docs/topics/impala_string_functions.xml      | 36 ++++++++++++++++++++++++++++
 tests/query_test/test_scanners.py            |  8 +++++++
 8 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 173ee632a..8daceb7f1 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -11098,6 +11098,17 @@ TEST_P(ExprTest, Utf8Test) {
   TestValue("utf8_length('你好 hello 你好')", TYPE_INT, 11);
   TestValue("utf8_length('hello')", TYPE_INT, 5);
 
+  // Verifies is_valid_utf8() validates bytes. The invalid sequences use unhex() since
+  // they cannot be written as string literals in this UTF-8 encoded file.
+  TestIsNull("is_valid_utf8(NULL)", TYPE_BOOLEAN);
+  TestValue<bool>("is_valid_utf8('')", TYPE_BOOLEAN, true); // empty is valid
+  TestValue<bool>("is_valid_utf8('hello')", TYPE_BOOLEAN, true); // ascii
+  TestValue<bool>("is_valid_utf8('你好')", TYPE_BOOLEAN, true); // 3-byte chars
+  TestValue<bool>("is_valid_utf8(unhex('ff'))", TYPE_BOOLEAN, false); // never valid
+  TestValue<bool>("is_valid_utf8(unhex('ab'))", TYPE_BOOLEAN, false); // lone cont. byte
+  TestValue<bool>("is_valid_utf8(unhex('c328'))", TYPE_BOOLEAN, false); // bad 2nd byte
+  TestValue<bool>("is_valid_utf8(unhex('eda080'))", TYPE_BOOLEAN, false); // surrogate
+
   // Verifies position and length of utf8_substring() are UTF-8 aware.
   // '你' and '好' are both encoded into 3 bytes.
   TestStringValue("utf8_substring('Hello', 1)", "Hello");
diff --git a/be/src/exprs/string-functions-ir.cc b/be/src/exprs/string-functions-ir.cc
index b179569f2..cba110bce 100644
--- a/be/src/exprs/string-functions-ir.cc
+++ b/be/src/exprs/string-functions-ir.cc
@@ -38,6 +38,7 @@
 #include "util/string-util.h"
 #include "util/ubsan.h"
 #include "util/url-parser.h"
+#include "util/utf8-util.h"
 
 #include "common/names.h"
 
@@ -290,6 +291,13 @@ IntVal StringFunctions::Utf8Length(FunctionContext* context, const StringVal& st
   return IntVal(CountUtf8Chars(str.ptr, str.len));
 }
 
+// Validates bytes, not characters, so it does not depend on UTF8_MODE.
+BooleanVal StringFunctions::IsValidUtf8Builtin(
+    FunctionContext* context, const StringVal& str) {
+  if (str.is_null) return BooleanVal::null();
+  return BooleanVal(IsValidUtf8(reinterpret_cast<const char*>(str.ptr), str.len));
+}
+
 // Marks as IR_ALWAYS_INLINE since this is called in MaskFunctions::MaskHash().
 // So the caller can also replace GetConstFnAttr() calls in codegen.
 IR_ALWAYS_INLINE StringVal StringFunctions::Lower(FunctionContext* context,
diff --git a/be/src/exprs/string-functions.h b/be/src/exprs/string-functions.h
index 8636d54f6..cce040965 100644
--- a/be/src/exprs/string-functions.h
+++ b/be/src/exprs/string-functions.h
@@ -108,6 +108,8 @@ class StringFunctions {
   static IntVal Length(FunctionContext*, const StringVal& str);
   static IntVal CharLength(FunctionContext*, const StringVal& str);
   static IntVal Utf8Length(FunctionContext*, const StringVal& str);
+  /// Returns true if 'str' is valid UTF-8. Not affected by UTF8_MODE.
+  static BooleanVal IsValidUtf8Builtin(FunctionContext*, const StringVal& str);
   static StringVal Lower(FunctionContext*, const StringVal& str);
   static StringVal LowerAscii(FunctionContext*, const StringVal& str);
   static StringVal LowerUtf8(FunctionContext*, const StringVal& str);
diff --git a/common/function-registry/impala_functions.py b/common/function-registry/impala_functions.py
index 69663b33f..446ca542b 100644
--- a/common/function-registry/impala_functions.py
+++ b/common/function-registry/impala_functions.py
@@ -536,6 +536,7 @@ visible_functions = [
   [['char_length'], 'INT', ['STRING'], 'impala::StringFunctions::Length'],
   [['character_length'], 'INT', ['STRING'], 'impala::StringFunctions::Length'],
   [['utf8_length'], 'INT', ['STRING'], 'impala::StringFunctions::Utf8Length'],
+  [['is_valid_utf8'], 'BOOLEAN', ['STRING'], 'impala::StringFunctions::IsValidUtf8Builtin'],
   [['lower', 'lcase'], 'STRING', ['STRING'], 'impala::StringFunctions::Lower'],
   [['lower_ascii', 'lcase_ascii'], 'STRING', ['STRING'], 'impala::StringFunctions::LowerAscii'],
   [['lower_utf8', 'lcase_utf8'], 'STRING', ['STRING'], 'impala::StringFunctions::LowerUtf8'],
diff --git a/common/thrift/generate_error_codes.py b/common/thrift/generate_error_codes.py
index 7c14452d3..8e7ab6f45 100755
--- a/common/thrift/generate_error_codes.py
+++ b/common/thrift/generate_error_codes.py
@@ -514,7 +514,8 @@ error_codes = (
    "(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.")
+   "columns as UTF-8, so a BINARY column is required there. To find the offending rows, "
+   "run SELECT * FROM <table> WHERE NOT is_valid_utf8($0).")
 )
 
 import sys
diff --git a/docs/topics/impala_incompatible_changes.xml b/docs/topics/impala_incompatible_changes.xml
index b1bafcea9..404f327ec 100644
--- a/docs/topics/impala_incompatible_changes.xml
+++ b/docs/topics/impala_incompatible_changes.xml
@@ -79,7 +79,10 @@ under the License.
             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>
+            required there.</p><p>To find the affected rows, use the
+            <xref href="impala_string_functions.xml#string_functions/is_valid_utf8">IS_VALID_UTF8()</xref>
+            function, for example
+            <codeph>SELECT * FROM t WHERE NOT is_valid_utf8(s)</codeph>.</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
diff --git a/docs/topics/impala_string_functions.xml b/docs/topics/impala_string_functions.xml
index 59ae62b69..5c262572f 100644
--- a/docs/topics/impala_string_functions.xml
+++ b/docs/topics/impala_string_functions.xml
@@ -130,6 +130,10 @@ under the License.
         <xref href="#string_functions/instr">INSTR</xref>
       </li>
 
+      <li>
+        <xref href="#string_functions/is_valid_utf8">IS_VALID_UTF8</xref>
+      </li>
+
       <li>
         <xref href="#string_functions/jaro_distance">JARO_DISTANCE, JARO_DIST</xref>
       </li>
@@ -696,6 +700,38 @@ select instr('foo bar bletch', 'b', 1, 2);
 
       </dlentry>
 
+      <dlentry id="is_valid_utf8" rev="IMPALA-15411">
+
+        <dt>
+          IS_VALID_UTF8(STRING str)
+        </dt>
+
+        <dd>
+          <b>Purpose:</b> Returns <codeph>true</codeph> if the argument is a valid UTF-8 encoded
+          string, <codeph>false</codeph> otherwise. Returns <codeph>NULL</codeph> if the argument
+          is <codeph>NULL</codeph>. An empty string is valid.
+          <p>
+            <b>Return type:</b> <codeph>BOOLEAN</codeph>
+          </p>
+
+          <p>
+            <b>Added in:</b> <keyword keyref="impala50"/>
+          </p>
+
+          <p conref="../shared/impala_common.xml#common/usage_notes_blurb"/>
+
+          <p>
+            This function validates bytes rather than characters, so its result does not depend on
+            the <codeph>UTF8_MODE</codeph> query option. It applies the same check as the Parquet
+            writer, so it can be used to locate values that would be rejected by the UTF-8
+            validation controlled by <xref keyref="parquet_annotate_strings_utf8"/>:
+          </p>
+
+<codeblock>SELECT * FROM t WHERE NOT is_valid_utf8(s);</codeblock>
+        </dd>
+
+      </dlentry>
+
       <dlentry id="jaro_distance">
 
         <dt>
diff --git a/tests/query_test/test_scanners.py b/tests/query_test/test_scanners.py
index f5e5a0743..196bc6929 100644
--- a/tests/query_test/test_scanners.py
+++ b/tests/query_test/test_scanners.py
@@ -1000,6 +1000,7 @@ class TestParquet(ImpalaTestSuite):
     err = self.execute_query_expect_failure(self.client, invalid,
         vector.get_value('exec_option'))
     assert "not valid UTF-8" in str(err)
+    assert "is_valid_utf8" in str(err)
 
     # The failed INSERT OVERWRITE must not have altered the table.
     result = self.execute_query("select s from %s" % qualified_table_name,
@@ -1015,6 +1016,13 @@ class TestParquet(ImpalaTestSuite):
     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)
+    self.execute_query("insert into %s values ('hello')" % src_tbl, opts)
+
+    # is_valid_utf8() finds the rows that fail the write-path validation.
+    result = self.execute_query(
+        "select count(*) from %s where not is_valid_utf8(s)" % src_tbl,
+        vector.get_value('exec_option'))
+    assert result.data == ['1']
 
     err = self.execute_query_expect_failure(self.client,
         "insert overwrite %s select s from %s" % (qualified_table_name, src_tbl),


(impala) 01/02: IMPALA-15422: Fix fall-through in system table DECIMAL writer

Posted by cs...@apache.org.
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 66db905e078d0962517f7e7982a18c0e6ee3f2bc
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Fri Sep 25 13:01:02 2026 +0200

    IMPALA-15422: Fix fall-through in system table DECIMAL writer
    
    The switch in WriteDecimalSlot() (system-table-scanner.cc) had no
    break statements, so after writing the value with the correct width
    it also wrote it as a Decimal16Value to the same address. All DECIMAL
    columns of sys.impala_query_live are DECIMAL(18,3), i.e. 8 bytes
    wide, so every write also overwrote the next 8 bytes of the tuple.
    Depending on the selected columns, this zeroed other slots or null
    indicators. E.g. 'select backends_count, total_time_ms from
    sys.impala_query_live' returned 0 for backends_count.
    
    This patch adds the missing break statements.
    
    Testing:
     - Added a custom cluster test that selects backends_count together
       with each DECIMAL column of sys.impala_query_live. It failed
       without the fix.
    
    Change-Id: I395a217c443f52d3588e0d25bdb1859fb245d04c
    Assisted-by: Claude Opus 5.5 (1M context) <no...@anthropic.com>
    Reviewed-on: http://gerrit.cloudera.org:8080/24948
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exec/system-table-scanner.cc     |  5 +++++
 tests/custom_cluster/test_query_live.py | 24 ++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/be/src/exec/system-table-scanner.cc b/be/src/exec/system-table-scanner.cc
index 229ad560c..e50309c2f 100644
--- a/be/src/exec/system-table-scanner.cc
+++ b/be/src/exec/system-table-scanner.cc
@@ -105,12 +105,17 @@ static void WriteDecimalSlot(
     case 4:
       *reinterpret_cast<Decimal4Value*>(slot) =
           Decimal4Value::FromDouble(type, value, false, &overflow);
+      break;
     case 8:
       *reinterpret_cast<Decimal8Value*>(slot) =
           Decimal8Value::FromDouble(type, value, false, &overflow);
+      break;
     case 16:
       *reinterpret_cast<Decimal16Value*>(slot) =
           Decimal16Value::FromDouble(type, value, false, &overflow);
+      break;
+    default:
+      DCHECK(false) << "Invalid decimal byte size: " << type.GetByteSize();
   }
   DCHECK(!overflow);
 }
diff --git a/tests/custom_cluster/test_query_live.py b/tests/custom_cluster/test_query_live.py
index 8eb4d919a..805f4036b 100644
--- a/tests/custom_cluster/test_query_live.py
+++ b/tests/custom_cluster/test_query_live.py
@@ -236,6 +236,30 @@ class TestQueryLive(CustomClusterTestSuite):
     assert_query('sys.impala_query_live', self.client, 'test_query_live',
                  result.runtime_profile)
 
+  @CustomClusterTestSuite.with_args(impalad_args="--query_log_write_interval_s=300 "
+                                                 "--cluster_id=test_query_live",
+                                    workload_mgmt=True,
+                                    disable_log_buffering=True)
+  def test_decimal_columns(self):
+    """IMPALA-15422: Writing a DECIMAL column must not overwrite other slots.
+       BACKENDS_COUNT is the only INT column, so it is laid out right after the 8-byte
+       DECIMAL slots, and it is written before them when it precedes them in the select
+       list."""
+    result = self.client.execute("select * from functional.alltypes")
+    describe = self.client.execute("describe sys.impala_query_live")
+    decimal_cols = [line.split('\t')[0] for line in describe.data
+                    if line.split('\t')[1].startswith('decimal')]
+    assert len(decimal_cols) > 0
+    expected = self.client.execute("select backends_count from sys.impala_query_live "
+        "where query_id='{}'".format(result.query_id))
+    assert expected.data == ["3"]
+    for col in decimal_cols:
+      actual = self.client.execute("select backends_count, {} from "
+          "sys.impala_query_live where query_id='{}'".format(col, result.query_id))
+      assert len(actual.data) == 1
+      assert actual.data[0].split('\t')[0] == "3", \
+          "backends_count overwritten when selected with {}".format(col)
+
   @CustomClusterTestSuite.with_args(impalad_args="--query_log_write_interval_s=300 "
                                                  "--cluster_id=test_query_live",
                                     workload_mgmt=True,