Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14197 )

Change subject: IMPALA-5092 Add support for VARCHAR in Kudu tables
......................................................................


Patch Set 14:

(3 comments)

The code looks good to me, but I would prefer to add some testing for all 
features added in this test, e.g. for partitioning on varchar(N)

http://gerrit.cloudera.org:8080/#/c/14197/14//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14197/14//COMMIT_MSG@27
PS14, Line 27: IMPALA-5675 tracks adding UTF-8 Character length support to 
VARCHAR
             : columns and marked the truncation code with a TODO that 
references
             : that Jira.
One more thing came up related to UTF-8: does it affect the ordering of 
strings? e.g. is it possible that s1 < s2 according to Impala, but as they 
contain multybyte characters, their ordering is different in Kudu? This could 
cause proplems in predicate push down.

I don't think that we can do much if this issue exists, but at least we should 
note somewhere that it is a known issue.


http://gerrit.cloudera.org:8080/#/c/14197/14//COMMIT_MSG@33
PS14, Line 33: * Manually reproduced a check failure due to multi-byte 
characters
             :   and tested that length truncation resolve that issue.
If this test is very hard to integrate into the Impala environment, then I am 
ok with moving that to a follow up jira, but I think that we should test this 
in Impala in the long term.

If the code to test it is not too complex, it would be great to add it here or 
into the follow up jira.


http://gerrit.cloudera.org:8080/#/c/14197/14//COMMIT_MSG@47
PS14, Line 47: support
What is the current state of min/max runtime filters for varchars? Are they 
turned off?



--
To view, visit http://gerrit.cloudera.org:8080/14197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d4959410fdd882bfa980cb55e8a7837c7823da8
Gerrit-Change-Number: 14197
Gerrit-PatchSet: 14
Gerrit-Owner: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Mar 2020 19:41:32 +0000
Gerrit-HasComments: Yes

Reply via email to