clickhouse-go icon indicating copy to clipboard operation
clickhouse-go copied to clipboard

Custom proto revision

Open titanproger opened this issue 7 months ago • 4 comments

Summary

Add ability to setup in options preferable Native TCP protocol version, to optimise data size, downloaded from server.

https://github.com/ClickHouse/clickhouse-go/discussions/1552

Checklist

Delete items not relevant to your PR:

  • [x] Unit and integration tests covering the common scenarios were added
  • [x] A human-readable description of the changes was provided to include in CHANGELOG - comment: not sure should i change CHANGELOG without version changing - it looks like generated automatically by some script
  • [x] For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials - comment: not a significant change

titanproger avatar May 07 '25 19:05 titanproger

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 07 '25 19:05 CLAassistant

As a justification that the library is supporting different revisions of protocol: here revision can be downgraded from server side

titanproger avatar May 08 '25 07:05 titanproger

Hey! I reached out to your team in Slack to see if we can get some more details about your use case. But until that channel is available I'll make some notes here:

Why would you want to change the protocol version without also adding the features in the code? I fear this will break client compatibility. If it were safe to override the client revision, why can't we simply update the client revision?

I followed the link in the discussion as well, the linked line didn't seem relevant for what you were referring to. I think I need more context. I don't want to expose your implementation details on a public PR so feel free to reach out in the Slack channel.

Thanks! 😄

SpencerTorres avatar May 14 '25 20:05 SpencerTorres

Hey! I reached out to your team in Slack to see if we can get some more details about your use case. But until that channel is available I'll make some notes here:

Why would you want to change the protocol version without also adding the features in the code? I fear this will break client compatibility. If it were safe to override the client revision, why can't we simply update the client revision?

I followed the link in the discussion as well, the linked line didn't seem relevant for what you were referring to. I think I need more context. I don't want to expose your implementation details on a public PR so feel free to reach out in the Slack channel.

Thanks! 😄

Thanks for feedback, @SpencerTorres ! let me provide some context:

Why would you want to change the protocol version without also adding the features in the code?

It is not allowed to set protocol version bigger than actually supported by client, or server It is all about decrease protocol version and disabling unwanted features.

I fear this will break client compatibility.

it should not - as, by default, if not specify desire proto version in options, it will use latest protocol version - as it was before this PR

If it were safe to override the client revision, why can't we simply update the client revision?

it is all about downgrade version, not upgrade version. Downgrade client should be avoided , also there is no 2.* version of client that uses minimal protocol version.

I followed the link in the discussion as well, the linked line didn't seem relevant for what you were referring to. I think I need more context.

Basically, the reason why i want to be able downgrade protocol version, is to reduce unwanted metadata downloaded from server. And, i think, if setup version protocol is available for ch-go library users, it will be good to also support it in clickhouse-go client. (by the way maybe it is good idea to name this option with same name - like ProtocolVersion, or NativeProtocolVersion to emphasise it for native proto only)

titanproger avatar May 15 '25 11:05 titanproger