Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/23485 )
Change subject: KUDU-1261 Add array type support to Python client ...................................................................... Patch Set 1: (4 comments) Thank you for taking care of the fixes and pushing it. Will post a patch shortly with the rest of the todos. Thank you Alexey! http://gerrit.cloudera.org:8080/#/c/23485/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23485/1//COMMIT_MSG@27 PS1, Line 27: - Decimal arrays: blocked on C++ API int128 overload support : (existing decimal uses int128, but SetArrayDecimal only has : int32/int64 overloads - needs API design review) > Is it a practical restriction (i.e. something Cython-related) or that's som Initially I thought that there would be some problem with the function name overloads. Turns out we can do it, will post decimal support in the followup patch. http://gerrit.cloudera.org:8080/#/c/23485/1/python/kudu/client.pyx File python/kudu/client.pyx: http://gerrit.cloudera.org:8080/#/c/23485/1/python/kudu/client.pyx@3234 PS1, Line 3234: value > Is this supposed to be Python list in case of arrays? Yes Python lists/tuples should work. Python Lists are beneficial as they handle null values (None) very seamlessly. Arrays have type limitations(we could not support all existing Kudu Python types with arrays.) and do not handle nulls. I would say arrays could be used as an extension for numeric arrays, which are larger to be a bit more performant. I created KUDU-3703 to track this. We can look into this in the future/implement it as we get more feedback as time allows etc. http://gerrit.cloudera.org:8080/#/c/23485/1/python/kudu/tests/test_schema.py File python/kudu/tests/test_schema.py: http://gerrit.cloudera.org:8080/#/c/23485/1/python/kudu/tests/test_schema.py@628 PS1, Line 628: self.assertEqual(col.type.name, 'nested') > nit: in C++ tests there also verification for the type of array elements (a Yes definitely will do. http://gerrit.cloudera.org:8080/#/c/23485/1/python/kudu/tests/test_schema.py@683 PS1, Line 683: empty_test_data = [ : ('arr_int8', []), : ('arr_int16', []), : ('arr_int32', []), : ('arr_int64', []), : ('arr_float', []), : ('arr_double', []), : ('arr_bool', []), : ('arr_string', []), : ('arr_binary', []), : ('arr_unixtime_micros', []), : ('arr_date', []), : ('arr_varchar', []), : ] > nit: does it make sense to add a similar bunch, but with nulls instead of e Yes sure. -- To view, visit http://gerrit.cloudera.org:8080/23485 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2329c7466534bd4961860c05b600e7d4b4a11507 Gerrit-Change-Number: 23485 Gerrit-PatchSet: 1 Gerrit-Owner: Marton Greber <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 06 Oct 2025 17:05:10 +0000 Gerrit-HasComments: Yes
