Skip to content

Conversation

OrionNebula
Copy link

Summary

If a user specifies the "readonly" option at the client level and tries to execute a query too long to fit in the GET request's query parameters, "readonly" is set twice in the header - once as "1" and again as whatever the user specified. In particular, this means it's not possible to set another value for "readonly" on queries that use the POST method. Bypass setting "readonly" the first time if it's part of the Client's options already.

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided so that we can include it in CHANGELOG later

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Olivia Trewin seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

.await
.unwrap();

assert_eq!(got_string, long_string);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally assert would be that readonly came through as 2. Changing query to show readonly could have that effect


let (method, body, content_length) = if use_post {
if read_only {
if read_only && !self.client.options.contains_key("readonly") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if read-only=0 is provided? According to https://clickhouse.com/docs/en/operations/settings/permissions-for-queries#readonly, we must use POST for such queries, so it should also affect the use_post flag.

@slvrtrn
Copy link
Contributor

slvrtrn commented Jun 9, 2025

Closing it for now. Let's discuss it in: #230

@slvrtrn slvrtrn closed this Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants