[ 
https://issues.apache.org/jira/browse/IMPALA-15423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Zoltán Borók-Nagy resolved IMPALA-15423.
----------------------------------------
    Fix Version/s: Impala 5.0.0
       Resolution: Fixed

> Non-ASCII strings are truncated of Iceberg metadata tables
> ----------------------------------------------------------
>
>                 Key: IMPALA-15423
>                 URL: https://issues.apache.org/jira/browse/IMPALA-15423
>             Project: IMPALA
>          Issue Type: Bug
>          Components: Backend
>            Reporter: Zoltán Borók-Nagy
>            Assignee: Zoltán Borók-Nagy
>            Priority: Major
>              Labels: impala-iceberg
>             Fix For: Impala 5.0.0
>
>
> There are two problems in the string/binary path of IcebergRowReader
> (be/src/exec/iceberg-metadata/iceberg-row-reader.cc).
> h3. 1. Non-ASCII strings are truncated
> IcebergRowReader::WriteStringOrBinarySlot() copies 
> {{jbuffer_guard.get_size()}} bytes. For
> strings, JniBufferGuard<jstring>::create() in be/src/util/jni-util.cc 
> computes the size
> and the buffer from two different encodings:
> {code:cpp}
> size = env->GetStringLength(jbuffer);                // UTF-16 code units
> buffer = env->GetStringUTFChars(jbuffer, &is_copy);  // modified UTF-8 bytes
> {code}
> Every non-ASCII character takes more UTF-8 bytes than UTF-16 units, so the 
> copied value
> is cut short. For example, 'Zürich' (6 units, 7 bytes) comes back as 'Züric', 
> and CJK
> text loses 2 bytes per character. Also, modified UTF-8 is not standard UTF-8:
> supplementary characters are encoded as 6-byte surrogate pairs and NUL as C0 
> 80. Using
> the right length alone would therefore still return non-standard bytes for 
> emoji and
> similar characters.
> This affects every STRING column and map value read through the metadata 
> scanner, e.g.
> partition values in {{`partitions`}} and {{`files`}}, and lower/upper bounds 
> in
> readable_metrics.
> {code:sql}
> CREATE TABLE ice_utf8 (city STRING, n INT) PARTITIONED BY SPEC (city) STORED 
> AS ICEBERG;
> INSERT INTO ice_utf8 VALUES ('Zürich', 1), ('東京', 2), ('abc😀', 3), ('plain', 
> 4);
> SELECT `partition` FROM db.ice_utf8.`partitions`;
> SELECT f.`partition`.city, hex(f.`partition`.city) FROM db.ice_utf8.`files` f;
> {code}
> {noformat}
> partition          hex(city)       expected hex
> {"city":"Züric"}   5AC3BC726963    5AC3BC72696368
> {"city":"�"}       E69D            E69DB1E4BAAC     ('東京')
> {"city":"abc��"}   616263EDA0      616263F09F9880   ('abc😀')
> {"city":"plain"}   706C61696E      706C61696E
> {noformat}
> The data itself is correct: {{SELECT city FROM ice_utf8}} returns the full 
> values. The
> emoji row shows both problems: the value is cut short, and the bytes that 
> remain
> (ED A0 …) are the start of a modified-UTF-8 surrogate, not the standard F0 9F 
> 98 80.
> The other JniUtfCharGuard users (fe-support.cc, logging-support.cc) only use 
> get() as a
> NUL-terminated string, so as far as I can see this reader is the only caller 
> hit by the
> size mismatch.
> h3. 2. JNI local references leak per value
> * WriteStringOrBinarySlot() never deletes {{jbuffer}}: the jstring returned by
> toString(), or the byte[] from ConvertJavaByteBufferToByteArray().
> * WriteMapKeyAndValue() never deletes the {{key}} and {{value}} objects 
> returned by
> GetNextMapKeyAndValue().
> The fragment thread is a native thread attached to the JVM and pushes no 
> local frame.
> These references therefore live until the thread detaches, keeping one Java 
> object
> reachable for every string/binary value and map entry of the whole scan. On
> {{`files`}}, {{`entries`}} and {{`all_*`}} tables of large tables, this pins 
> a lot of heap
> in the coordinator's JVM.
> Measured on a 1824-file table (a CTAS of tpcds_parquet.store_sales 
> partitioned by
> ss_sold_date_sk, 23 columns). The query was
> {{select file_path, lower_bounds, upper_bounds, column_sizes, value_counts 
> from t.`files`}},
> with SPOOL_QUERY_RESULTS=false and the client blocked so the fragment stays 
> open. Live
> objects in the coordinator JVM ({{jcmd GC.class_histogram}}):
> ||Class||Idle||During the scan||After the query||
> |java.nio.HeapByteBuffer|3,662|87,566 (+83,904 = 1824 × 46 bound 
> values)|3,662|
> |java.lang.Long|656|82,762 (+82,106, the column_sizes/value_counts 
> values)|656|
> |[B|267,244|437,588 (+170,344)|267,387|
> Every map value the scan read stays live until the fragment thread exits. The 
> byte[]
> count covers both the ByteBuffers' backing arrays and the copies made by
> ConvertJavaByteBufferToByteArray(). A control query that reads only BIGINT 
> columns
> (record_count, file_size_in_bytes) shows no growth.
> h3. Proposed fix
> * Produce the bytes on the Java side with 
> {{String.getBytes(StandardCharsets.UTF_8)}} and
> read them through JniByteArrayGuard. Alternatively, fix 
> JniBufferGuard<jstring> to use
> GetStringUTFLength and document the modified-UTF-8 caveat.
> * DeleteLocalRef {{jbuffer}}, {{key}} and {{value}} after use, or push/pop a 
> JniLocalFrame
> per row in IcebergMetadataScanNode::GetNext().
> * Add EE tests with non-ASCII partition values, including a supplementary 
> character.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to