[jira] [Commented] (CALCITE-4367) Incorrect documentation for Avatica JSON request/response signatures
[
https://issues.apache.org/jira/browse/CALCITE-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17397562#comment-17397562
]
John Bodley commented on CALCITE-4367:
--
Per [~elserj]'s request created
[CALCITE-4727|https://issues.apache.org/jira/browse/CALCITE-4727] for tracking
the additional correction.
> Incorrect documentation for Avatica JSON request/response signatures
>
>
> Key: CALCITE-4367
> URL: https://issues.apache.org/jira/browse/CALCITE-4367
> Project: Calcite
> Issue Type: Bug
> Components: avatica
>Reporter: John Bodley
>Assignee: Josh Elser
>Priority: Trivial
> Fix For: avatica-1.18.0
>
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> I noticed a few inconsistencies between what is documented in the [Avatica
> JSON Reference|https://calcite.apache.org/avatica/docs/json_reference.html]
> and what the Avatica JDBC driver provides, specifically:
> # The {{DatabasePropertyRequest}} was missing the {{connection_id}} field in
> the example signature.
> # `RpcMetadata` is actually a response as opposed to a miscellaneous type per
> [here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
> and thus requires a {{response}} field. Note I'm not certain if this was
> intentional, i.e., it being a response, however it it is it seems that it
> should be renamed to {{RpcMetadataResponse}} for consistency.
> # The supplied {{ConnectionProperties}} contains an undocumented {{dirty}}
> field ({{is_dirty}} for protobuf).
> # For the {{SchemasRequest}} the {{catalog}} and {{schemaPattern}} are
> optional rather than required.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4367) Incorrect documentation for Avatica JSON request/response signatures
[
https://issues.apache.org/jira/browse/CALCITE-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17397499#comment-17397499
]
Josh Elser commented on CALCITE-4367:
-
[~john.bod...@gmail.com] thanks for coming back and keeping us honest on the
documentation. Could you open a new Jira issue for this extra correction? Happy
to get it fixed, but it helps us with tracking to get new issues filed (to
avoid difficult questions about when fixes were actually made).
> Incorrect documentation for Avatica JSON request/response signatures
>
>
> Key: CALCITE-4367
> URL: https://issues.apache.org/jira/browse/CALCITE-4367
> Project: Calcite
> Issue Type: Bug
> Components: avatica
>Reporter: John Bodley
>Assignee: Josh Elser
>Priority: Trivial
> Fix For: avatica-1.18.0
>
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> I noticed a few inconsistencies between what is documented in the [Avatica
> JSON Reference|https://calcite.apache.org/avatica/docs/json_reference.html]
> and what the Avatica JDBC driver provides, specifically:
> # The {{DatabasePropertyRequest}} was missing the {{connection_id}} field in
> the example signature.
> # `RpcMetadata` is actually a response as opposed to a miscellaneous type per
> [here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
> and thus requires a {{response}} field. Note I'm not certain if this was
> intentional, i.e., it being a response, however it it is it seems that it
> should be renamed to {{RpcMetadataResponse}} for consistency.
> # The supplied {{ConnectionProperties}} contains an undocumented {{dirty}}
> field ({{is_dirty}} for protobuf).
> # For the {{SchemasRequest}} the {{catalog}} and {{schemaPattern}} are
> optional rather than required.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4367) Incorrect documentation for Avatica JSON request/response signatures
[
https://issues.apache.org/jira/browse/CALCITE-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17397465#comment-17397465
]
John Bodley commented on CALCITE-4367:
--
[~elserj] and [~francischuang] a follow up regarding the RpcMetadata, per
[https://github.com/apache/calcite-avatica/pull/129] and my experience using
the Avatica JDBC connector with Tableau, for the JSON objects the
[rpcMetadata|https://calcite.apache.org/avatica/docs/json_reference.html#rpcmetadata]
does expect a `response` field. This hypothesis is potentially confirmed
[here|https://github.com/apache/calcite-avatica/blob/89e0deb510311b85b8c8bacde6d2ff70c309930e/core/src/main/java/org/apache/calcite/avatica/remote/Service.java#L2674].
> Incorrect documentation for Avatica JSON request/response signatures
>
>
> Key: CALCITE-4367
> URL: https://issues.apache.org/jira/browse/CALCITE-4367
> Project: Calcite
> Issue Type: Bug
> Components: avatica
>Reporter: John Bodley
>Assignee: Josh Elser
>Priority: Trivial
> Fix For: avatica-1.18.0
>
> Time Spent: 2h 10m
> Remaining Estimate: 0h
>
> I noticed a few inconsistencies between what is documented in the [Avatica
> JSON Reference|https://calcite.apache.org/avatica/docs/json_reference.html]
> and what the Avatica JDBC driver provides, specifically:
> # The {{DatabasePropertyRequest}} was missing the {{connection_id}} field in
> the example signature.
> # `RpcMetadata` is actually a response as opposed to a miscellaneous type per
> [here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
> and thus requires a {{response}} field. Note I'm not certain if this was
> intentional, i.e., it being a response, however it it is it seems that it
> should be renamed to {{RpcMetadataResponse}} for consistency.
> # The supplied {{ConnectionProperties}} contains an undocumented {{dirty}}
> field ({{is_dirty}} for protobuf).
> # For the {{SchemasRequest}} the {{catalog}} and {{schemaPattern}} are
> optional rather than required.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4367) Incorrect documentation for Avatica JSON request/response signatures
[
https://issues.apache.org/jira/browse/CALCITE-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17254361#comment-17254361
]
Josh Elser commented on CALCITE-4367:
-
Pull request is up. If I don't get a review, I'll plan on committing this in a
few days.
> Incorrect documentation for Avatica JSON request/response signatures
>
>
> Key: CALCITE-4367
> URL: https://issues.apache.org/jira/browse/CALCITE-4367
> Project: Calcite
> Issue Type: Bug
> Components: avatica
>Reporter: John Bodley
>Assignee: Josh Elser
>Priority: Trivial
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> I noticed a few inconsistencies between what is documented in the [Avatica
> JSON Reference|https://calcite.apache.org/avatica/docs/json_reference.html]
> and what the Avatica JDBC driver provides, specifically:
> # The {{DatabasePropertyRequest}} was missing the {{connection_id}} field in
> the example signature.
> # `RpcMetadata` is actually a response as opposed to a miscellaneous type per
> [here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
> and thus requires a {{response}} field. Note I'm not certain if this was
> intentional, i.e., it being a response, however it it is it seems that it
> should be renamed to {{RpcMetadataResponse}} for consistency.
> # The supplied {{ConnectionProperties}} contains an undocumented {{dirty}}
> field ({{is_dirty}} for protobuf).
> # For the {{SchemasRequest}} the {{catalog}} and {{schemaPattern}} are
> optional rather than required.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4367) Incorrect documentation for Avatica JSON request/response signatures
[
https://issues.apache.org/jira/browse/CALCITE-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17254357#comment-17254357
]
Josh Elser commented on CALCITE-4367:
-
{quote}
`RpcMetadata` is actually a response as opposed to a miscellaneous type per
[here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
and thus requires a {{response}} field. Note I'm not certain if this was
intentional, i.e., it being a response, however it it is it seems that it
should be renamed to {{RpcMetadataResponse}} for consistency.
It's intentional that it isn't a response, as a "Response" was intended to be
top-level messages which might be sent back by the Avatica server. However,
it's not used by any requests, so it lives in responses.proto to avoid
polluting common.proto (for no reason). Hope this makes sense. I'll update the
comment to reflect this.
bq. The supplied ConnectionProperties contains an undocumented dirty field
(is_dirty for protobuf).
Good catch. Another funny one -- this shouldn't ever be sent over the wire.
It's just internal state used to avoid unnecessary RPCs (to sync the client's
properties with the properties in the Avatica server). I'll update the docs for
now, but we should circle back to avoid this getting serialized at all (for
json and proto)
The rest of these were spot-on. Thanks for the easy-to-incorporate feedback,
[~john.bod...@gmail.com]
> Incorrect documentation for Avatica JSON request/response signatures
>
>
> Key: CALCITE-4367
> URL: https://issues.apache.org/jira/browse/CALCITE-4367
> Project: Calcite
> Issue Type: Bug
> Components: avatica
>Reporter: John Bodley
>Assignee: Josh Elser
>Priority: Trivial
> Time Spent: 20m
> Remaining Estimate: 0h
>
> I noticed a few inconsistencies between what is documented in the [Avatica
> JSON Reference|https://calcite.apache.org/avatica/docs/json_reference.html]
> and what the Avatica JDBC driver provides, specifically:
> # The {{DatabasePropertyRequest}} was missing the {{connection_id}} field in
> the example signature.
> # `RpcMetadata` is actually a response as opposed to a miscellaneous type per
> [here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
> and thus requires a {{response}} field. Note I'm not certain if this was
> intentional, i.e., it being a response, however it it is it seems that it
> should be renamed to {{RpcMetadataResponse}} for consistency.
> # The supplied {{ConnectionProperties}} contains an undocumented {{dirty}}
> field ({{is_dirty}} for protobuf).
> # For the {{SchemasRequest}} the {{catalog}} and {{schemaPattern}} are
> optional rather than required.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (CALCITE-4367) Incorrect documentation for Avatica JSON request/response signatures
[
https://issues.apache.org/jira/browse/CALCITE-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17249864#comment-17249864
]
Josh Elser commented on CALCITE-4367:
-
Thanks for reporting, John. Will see if we can get the docs updated.
> Incorrect documentation for Avatica JSON request/response signatures
>
>
> Key: CALCITE-4367
> URL: https://issues.apache.org/jira/browse/CALCITE-4367
> Project: Calcite
> Issue Type: Bug
> Components: avatica
>Reporter: John Bodley
>Priority: Trivial
> Time Spent: 20m
> Remaining Estimate: 0h
>
> I noticed a few inconsistencies between what is documented in the [Avatica
> JSON Reference|https://calcite.apache.org/avatica/docs/json_reference.html]
> and what the Avatica JDBC driver provides, specifically:
> # The {{DatabasePropertyRequest}} was missing the {{connection_id}} field in
> the example signature.
> # `RpcMetadata` is actually a response as opposed to a miscellaneous type per
> [here|https://github.com/apache/calcite-avatica/blob/4b7eee5bf430b916c7c07897b6f60d2b6b6dabb7/core/src/main/protobuf/responses.proto#L114-L116]
> and thus requires a {{response}} field. Note I'm not certain if this was
> intentional, i.e., it being a response, however it it is it seems that it
> should be renamed to {{RpcMetadataResponse}} for consistency.
> # The supplied {{ConnectionProperties}} contains an undocumented {{dirty}}
> field ({{is_dirty}} for protobuf).
> # For the {{SchemasRequest}} the {{catalog}} and {{schemaPattern}} are
> optional rather than required.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
