Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23493 )

Change subject: IMPALA-14472: Add create/read support for ARRAY column of Kudu
......................................................................


Patch Set 4:

(5 comments)

Thank you, Csaba. I can answer your question about the pytest. Will ask around 
for the others.

http://gerrit.cloudera.org:8080/#/c/23493/4/fe/src/main/java/org/apache/impala/util/KuduUtil.java
File fe/src/main/java/org/apache/impala/util/KuduUtil.java:

http://gerrit.cloudera.org:8080/#/c/23493/4/fe/src/main/java/org/apache/impala/util/KuduUtil.java@425
PS4, Line 425: public static org.apache.kudu.Type
nit: Can use @Nullable annotation here.


http://gerrit.cloudera.org:8080/#/c/23493/4/fe/src/main/java/org/apache/impala/util/KuduUtil.java@461
PS4, Line 461:
             :   public static ScalarType toImpalaScalarType
nit: Method can use some documentation.


http://gerrit.cloudera.org:8080/#/c/23493/4/fe/src/main/java/org/apache/impala/util/KuduUtil.java@486
PS4, Line 486:
             :   public static Type toImpalaType
nit: Method can use some documentation.


http://gerrit.cloudera.org:8080/#/c/23493/4/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:

http://gerrit.cloudera.org:8080/#/c/23493/4/tests/query_test/test_kudu.py@1868
PS4, Line 1868: functional_kudu
> Why functional_kudu, can't the test use a unique_database? This could cause
test_unsupported_types is a negative tests, so this DDL will not succeed.
Because this is a negative test, it is unnecessary to create unique_database 
for nothing.


http://gerrit.cloudera.org:8080/#/c/23493/4/tests/query_test/test_kudu.py@1966
PS4, Line 1966:     expected_tuples = list(zip(
              :         range(len(sorted_tuples)),
              :         *[EXPECTED_COLUMNS[item_type] for item_type in types]
              :     ))
> Wouldn't it be easier to use a .test file? SUPPORTED_ITEM_TYPES seems const
Since VARCHAR(1) testcase use UTF-8 chars, it is only possible to validate it 
here inside python code.
Our test infrastructure is not capable in parsing .test file with UTF-8 chars 
in it yet.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9282aac821bd30668189f84b2ed8fff7047e7310
Gerrit-Change-Number: 23493
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Xuebin Su <[email protected]>
Gerrit-Comment-Date: Tue, 07 Oct 2025 17:28:08 +0000
Gerrit-HasComments: Yes

Reply via email to