HIVE-27370: support 4 bytes characters #5624
Conversation
|
|
@difin, please take a look when you have time |
|
@deniskuzZ Sorry, I also overlooked this comments. I fixed it. |
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
|
Sure. I was not confident that I should review coworker's patch to avoid bias. I will check this anyway. |
| substr('あa🤎いiうu', 1, 3) as b1, | ||
| substr('あa🤎いiうu', 3) as b2, | ||
| substr('あa🤎いiうu', -5) as b3 | ||
| FROM src tablesample (1 rows); |
There was a problem hiding this comment.
I verified the master branch can't pass this test case
+POSTHOOK: query: SELECT
+ substr('あa🤎いiうu', 1, 3) as b1,
+ substr('あa🤎いiうu', 3) as b2,
+ substr('あa🤎いiうu', -5) as b3
+FROM src tablesample (1 rows)
+POSTHOOK: type: QUERY
+POSTHOOK: Input: default@src
+#### A masked pattern was here ####
+あa? 🤎いiうu ?いiうu
| String s = t.toString(); | ||
| int[] index = makeIndex(pos, len, s.length()); | ||
| if (index == null) { | ||
| byte[] utf8String = t.toString().getBytes(); |
There was a problem hiding this comment.
Why not use Text#getBytes or Text#copyBytes? Probably, getBytes is preferable because we don't mutate it. Note that we have to use Text#getLength instead of the size of the byte array.
https://github.com/apache/hadoop/blob/rel/release-3.4.1/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/Text.java#L116-L132
There was a problem hiding this comment.
Ah, OK. I will fix it.
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
asf-ci-hive
added
tests pending
tests unstable
and removed
tests passed
tests pending
labels
Co-authored-by: Shohei Okumiya <[email protected]>
asf-ci-hive
added
tests pending
tests failed
and removed
tests unstable
tests pending
labels
|
Looks good to me, but we're running additional in-house integration tests just in case. |
|
What changes were proposed in this pull request?
If a
SUBSTRUDF has a 4-byte characters in its parameter, the behavior is different between vectorized and non-vectorized. The vectorized version handles 4-byte characters properly, but the non-vectorized version does not, so similar logic is needed.And these fixes use vectorized logic:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java#L89-L130
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java#L78-L109
Why are the changes needed?
Vectorized and non-vectorized have different results.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added pattern tests to itest for these to work correctly.