Skip to content

Implement query with interpolation flags - #259

Open
ozgurcancal wants to merge 1 commit into
ClickHouse:mainfrom
ozgurcancal:query_flags
Open

Implement query with interpolation flags#259
ozgurcancal wants to merge 1 commit into
ClickHouse:mainfrom
ozgurcancal:query_flags

Conversation

@ozgurcancal

Copy link
Copy Markdown

Summary

Add support for explicit interpolation flags to specify exactly which interpolation features should be enabled in the query.
Add integration tests for possible flag combinations.
Update README.

Reference: #157

Comment thread src/sql/mod.rs
Comment on lines +130 to +133
eprintln!(
"warning use QI::BIND template flag,bind() skipped for query: {}",
self
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's better to use the log crate here.

Suggested change
eprintln!(
"warning use QI::BIND template flag,bind() skipped for query: {}",
self
);
log::warn!("use QI::BIND template flag,bind() skipped for query: {self}");

@ozgurcancal ozgurcancal Jul 28, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks, This is the default macro stated in the macros.rs

Comment thread src/query/query.rs
#[must_use]
#[derive(Clone)]
pub struct Query {
pub struct Query<const INTERPFLAGS: u8 = { QI::DEFAULT }> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changing a public type should be a major version bump.

Can this be avoided, by the way? What is the purpose of having INTERPFLAGS here instead of a regular setter?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for your comment. I think discouraging unconscious use of bind or fields function should be query wise. Otherwise, it would appear less intuitive and introduce unnecessary complexity, since these functions are commonly used with both execute and fetch.
Therefore, handling interpolation only in fetch would not be sufficient. If fetch were the only place where interpolation was needed, I would simply enable all flags (including bind and fields) and update the interpolation state directly via the fetch function template argument.

@ozgurcancal

ozgurcancal commented Oct 5, 2025

Copy link
Copy Markdown
Author

I have fix the conflicts in the PR, we can implement the raw query on the top of it. what do you think? @slvrtrn @loyd

@ozgurcancal
ozgurcancal requested a review from pravic October 5, 2025 19:23
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.

3 participants