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

Change subject: WIP KUDU-1261 introduce nested types for ColumnSchemaPB
......................................................................


Patch Set 2:

(2 comments)

Thanks for the feedback!

http://gerrit.cloudera.org:8080/#/c/22925/1/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/22925/1/src/kudu/common/common.proto@73
PS1, Line 73: add the '
> I guess we would be having some conditions in place to ensure this can't be
In the first phase with supporting one-dimensional array of scalar types only 
-- yes, there will be a check in the implementation to accept only a scalar 
data type for this field.


http://gerrit.cloudera.org:8080/#/c/22925/2/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/22925/2/src/kudu/common/common.proto@152
PS2, Line 152:   optional bytes read_default_value = 6;
             :   optional bytes write_default_value = 7;
> Any thoughts on how to extend these to array datatype? Are we using the sam
As I understand, there is no need to extend these since their format is 
universal.  These are just bytes to interpret depending on the column type, and 
all the details of the interpretation are in the implementation, not in the 
interface.  Both the server and the client sides will need to use the same 
convention of interpreting these for nested types -- the same as they do now.

However, since there isn't a contract explicitly expressed in the API, we will 
need to use feature flags to track incompatibilities between the server and the 
client side when adding support for multi-dimensional arrays and other nested 
types (i.e. maps & structures).  As of 'scalar --> 1d scalar arrays' step now, 
it seems it's enough to have just the new NESTED enumerator to tell such an 
incompatibility.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf5f32c4291e7906ffb68070bee0d5369fab36fd
Gerrit-Change-Number: 22925
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 21 May 2025 19:31:47 +0000
Gerrit-HasComments: Yes

Reply via email to