[
https://issues.apache.org/jira/browse/CALCITE-1353?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15603458#comment-15603458
]
Josh Elser commented on CALCITE-1353:
-------------------------------------
bq. does this meet your requirements for backwards compatibility? If so I'll
commit and include in the 1.9 release.
{code}
- uint64 first_frame_max_size = 3; // The maximum number of rows to return in
the first Frame
+ int32 first_frame_max_size = 3; // The maximum number of rows to return in
the first Frame
{code}
Hurm. I actually don't know if protobuf can/will handle this. Since Java
doesn't differentiate between unsigned and signed types, that aspect is fine.
{{RemoteMeta#execute(...)}} is passing in an int, which is cast to a long,
which is then cast back to int (unnecessary much? :P). I think ExecuteRequest's
maxRowCount should really be made an int instead of a long with this changeset.
For languages that do differentiate between signed and unsigned types, I'm not
sure how those languages will handle this nor how the protobuf library for that
language will handle it. Do you have insight here with your experience writing
the Go driver, [~francischuang]? Does your client work unmodified against an
Avatica server with this change?
Commonly, when we (I) messed up the protobuf objects, the process is to add a
new field to the message, update the serialization methods to read/set both the
old and new field, and switch the java client to use the new field. That gives
us a path forward to eventually get everyone off of those deprecated fields. I
think we should definitely test this out with a client to see if it is
compatible before committing.
> first_frame_max_size in an ExecuteRequest should be an int32 in protobuf
> definitions.
> -------------------------------------------------------------------------------------
>
> Key: CALCITE-1353
> URL: https://issues.apache.org/jira/browse/CALCITE-1353
> Project: Calcite
> Issue Type: Bug
> Components: avatica
> Affects Versions: avatica-1.8.0
> Reporter: Francis Chuang
> Priority: Minor
> Fix For: avatica-1.10.0
>
>
> In the protobuf definition for {{ExecuteRequest}}, the
> {{first_frame_max_size}} parameter is typed as an {{uint64}}. See
> https://github.com/apache/calcite/blob/master/avatica/core/src/main/protobuf/requests.proto#L130.
> For consistency, it should be an {{int32}}.
> Similar parameters relating to the frame size are all typed as {{int32}}.
> For a {{PrepareAndExecuteRequest}}, {{first_frame_max_size}} is a{{int32}}:
> https://github.com/apache/calcite/blob/master/avatica/core/src/main/protobuf/requests.proto#L78
> For a {{FetchRequest}}, {{frame_max_size}} is a {{int32}}:
> https://github.com/apache/calcite/blob/master/avatica/core/src/main/protobuf/requests.proto#L96
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
