Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23494 )

Change subject: KUDU-1261 Python array datatype fixes
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

This looks good to me; thanks a lot for this follow-up patch!

http://gerrit.cloudera.org:8080/#/c/23494/1/python/kudu/client.pyx
File python/kudu/client.pyx:

http://gerrit.cloudera.org:8080/#/c/23494/1/python/kudu/client.pyx@3514
PS1, Line 3514:         elif elem_type == KUDU_DECIMAL:
              :             # Determine element precision/scale from schema
              :             py_col = self.schema[i]
              :             precision = py_col.type_attributes.precision
              :             scale = py_col.type_attributes.scale
              :             if precision <= 9:
              :                 cpp_values_int32.reserve(array_len)
              :                 for elem in value:
              :                     if elem is None:
              :                         cpp_values_int32.push_back(0)
              :                     else:
              :                         # Match scalar decimal semantics: rely 
on the value's own exponent
              :                         
cpp_values_int32.push_back(<int32_t>to_unscaled_decimal(elem))
              :                 
check_status(self.row.SetArrayUnscaledDecimal(i, cpp_values_int32, 
cpp_validity))
              :             elif precision <= 18:
              :                 cpp_values_int64.reserve(array_len)
              :                 for elem in value:
              :                     if elem is None:
              :                         cpp_values_int64.push_back(0)
              :                     else:
              :                         # Match scalar decimal semantics: rely 
on the value's own exponent
              :                         
cpp_values_int64.push_back(<int64_t>to_unscaled_decimal(elem))
              :                 
check_status(self.row.SetArrayUnscaledDecimal(i, cpp_values_int64, 
cpp_validity))
Thanks a lot for making this work with the current API for setting decimal 
arrays.

BTW, we can still change the interface of the Kudu C++ client, switching to as 
single method accepting vector<int128_t>, and do all the checks in the 
partial_raw.cc.  There will be more verification in partial_row.cc, and copying 
from an input array into a temporary array for int32_t and int64_t, but it's 
still
possible.

My concern was that it might be a waste using 128bit integers for 32bit ones 
for quite long arrays, but if current client C++ API looks so ugly that my 
concern seems minor in the scheme of things, we should consider switching to 
always using vector<int128_t> for unscaled decimals.

Thanks!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77292b07a854b4f48a75d4e1293f302e8bad9129
Gerrit-Change-Number: 23494
Gerrit-PatchSet: 1
Gerrit-Owner: Marton Greber <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 07 Oct 2025 06:38:17 +0000
Gerrit-HasComments: Yes

Reply via email to