Skip to content

Commit 911124d

Browse files
committed
query_interpolation conflict fix
1 parent 75bc386 commit 911124d

File tree

12 files changed

+432
-54
lines changed

12 files changed

+432
-54
lines changed

examples/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ If something is missing, or you found a mistake in one of these examples, please
99
### General usage
1010

1111
- [usage.rs](usage.rs) - creating tables, executing other DDLs, inserting the data, and selecting it back. Optional cargo features: `inserter`.
12+
- [query_flags.rs](query_flags.rs) - Supports query interpolation flags to finely control how SQL templates are processed.
1213
- [mock.rs](mock.rs) - writing tests with `mock` feature. Cargo features: requires `test-util`.
1314
- [inserter.rs](inserter.rs) - using the client-side batching via the `inserter` feature. Cargo features: requires `inserter`.
1415
- [async_insert.rs](async_insert.rs) - using the server-side batching via the [asynchronous inserts](https://clickhouse.com/docs/en/optimize/asynchronous-inserts) ClickHouse feature

src/lib.rs

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,40 @@ impl Client {
380380
inserter::Inserter::new(self, table)
381381
}
382382

383-
/// Starts a new SELECT/DDL query.
383+
/// Starts a new SELECT/DDL query with default interpolation flags.
384+
///
385+
/// This method uses [`query::QI::DEFAULT`] flags which enable only:
386+
/// - `?fields` substitution with struct field names
387+
///
388+
/// Parameter binding (`?`) is not enabled by default.
389+
/// For explicit control over interpolation features, use [`Client::query_with_flags`].
384390
pub fn query(&self, query: &str) -> query::Query {
385391
query::Query::new(self, query)
386392
}
387393

394+
/// Starts a new SELECT/DDL query with explicit interpolation flags
395+
/// to specify exactly which interpolation features should be enabled.
396+
///
397+
/// ## Example with both features enabled
398+
/// ```rust,ignore
399+
/// use clickhouse::query::QI;
400+
///
401+
/// let rows = client
402+
/// .query_with_flags::<{ QI::FIELDS | QI::BIND }>("SELECT ?fields FROM users WHERE age > ?")
403+
/// .bind(18)
404+
/// .fetch::<User>()
405+
/// .await?;
406+
/// ```
407+
///
408+
/// # Comparison with [`Client::query`]
409+
///
410+
/// - [`Client::query`] uses default flags ([`query::QI::DEFAULT`]) enabling only `?fields`
411+
/// - [`Client::query_with_flags`] allows explicit control over interpolation features
412+
///
413+
pub fn query_with_flags<const FLAGS: u8>(&self, query: &str) -> query::Query<FLAGS> {
414+
query::Query::<FLAGS>::new(self, query)
415+
}
416+
388417
/// Enables or disables [`Row`] data types validation against the database schema
389418
/// at the cost of performance. Validation is enabled by default, and in this mode,
390419
/// the client will use `RowBinaryWithNamesAndTypes` format.
@@ -737,3 +766,59 @@ mod client_tests {
737766
assert_ne!(client.options, client_clone.options,);
738767
}
739768
}
769+
770+
#[cfg(test)]
771+
mod query_flags_tests {
772+
use super::*;
773+
use crate::query::QI;
774+
775+
#[test]
776+
fn test_query_with_flags() {
777+
let client = Client::default();
778+
779+
// Test query with BIND flag
780+
let query_bind = client.query_with_flags::<{ QI::BIND }>("SELECT * FROM test WHERE id = ?");
781+
assert_eq!(
782+
format!("{}", query_bind.sql_display()),
783+
"SELECT * FROM test WHERE id = ?"
784+
);
785+
786+
// Test query with FIELDS flag
787+
let query_fields = client.query_with_flags::<{ QI::FIELDS }>("SELECT ?fields FROM test");
788+
assert_eq!(
789+
format!("{}", query_fields.sql_display()),
790+
"SELECT ?fields FROM test"
791+
);
792+
793+
// Test query with combined flags
794+
let query_combined = client
795+
.query_with_flags::<{ QI::FIELDS | QI::BIND }>("SELECT ?fields FROM test WHERE id = ?");
796+
assert_eq!(
797+
format!("{}", query_combined.sql_display()),
798+
"SELECT ?fields FROM test WHERE id = ?"
799+
);
800+
}
801+
802+
#[test]
803+
fn test_binding_behavior_with_flags() {
804+
let client = Client::default();
805+
806+
// Test with BIND flag - should work normally
807+
let mut query_with_bind =
808+
client.query_with_flags::<{ QI::BIND }>("SELECT * FROM test WHERE id = ?");
809+
query_with_bind = query_with_bind.bind(42);
810+
assert_eq!(
811+
format!("{}", query_with_bind.sql_display()),
812+
"SELECT * FROM test WHERE id = 42"
813+
);
814+
815+
// Test without BIND flag - should skip binding
816+
let mut query_without_bind =
817+
client.query_with_flags::<{ QI::NONE }>("SELECT * FROM test WHERE id = ?");
818+
query_without_bind = query_without_bind.bind(42); // This should be skipped
819+
assert_eq!(
820+
format!("{}", query_without_bind.sql_display()),
821+
"SELECT * FROM test WHERE id = ?"
822+
);
823+
}
824+
}

src/query/mod.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Suppress Clippy warning due to identical names for the folder and file (`query`)
2+
#![allow(clippy::module_inception)]
3+
4+
// Declare the submodules
5+
mod query;
6+
mod query_flags;
7+
8+
// Re-export public items
9+
pub use crate::cursors::{BytesCursor, RowCursor};
10+
pub use query::Query;
11+
pub use query_flags::QI;

src/query.rs renamed to src/query/query.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,25 @@ use crate::{
1515

1616
const MAX_QUERY_LEN_TO_USE_GET: usize = 8192;
1717

18-
pub use crate::cursors::{BytesCursor, RowCursor};
18+
pub(super) use crate::cursors::{BytesCursor, RowCursor};
1919
use crate::headers::with_authentication;
2020

21+
use crate::query::query_flags::QI;
22+
2123
#[must_use]
2224
#[derive(Clone)]
23-
pub struct Query {
25+
pub struct Query<const INTERPFLAGS: u8 = { QI::DEFAULT }> {
2426
client: Client,
2527
sql: SqlBuilder,
28+
interp_flags: u8,
2629
}
2730

28-
impl Query {
31+
impl<const INTERPFLAGS: u8> Query<INTERPFLAGS> {
2932
pub(crate) fn new(client: &Client, template: &str) -> Self {
3033
Self {
3134
client: client.clone(),
3235
sql: SqlBuilder::new(template),
36+
interp_flags: INTERPFLAGS,
3337
}
3438
}
3539

@@ -53,7 +57,16 @@ impl Query {
5357
/// [`Identifier`]: crate::sql::Identifier
5458
#[track_caller]
5559
pub fn bind(mut self, value: impl Bind) -> Self {
56-
self.sql.bind_arg(value);
60+
if QI::has_bind(self.interp_flags) {
61+
// Only bind if the BIND flag is set
62+
self.sql.bind_arg(value);
63+
} else {
64+
// Warn if QI::BIND flag is not set - binding will be skipped
65+
eprintln!(
66+
"warning: .bind() used without QI::BIND flag; use client.query_with_flags::<{{QI::BIND}}>(...) for binding: {}",
67+
self.sql
68+
);
69+
}
5770
self
5871
}
5972

@@ -84,7 +97,9 @@ impl Query {
8497
/// # Ok(()) }
8598
/// ```
8699
pub fn fetch<T: Row>(mut self) -> Result<RowCursor<T>> {
87-
self.sql.bind_fields::<T>();
100+
if QI::has_fields(self.interp_flags) {
101+
self.sql.bind_fields::<T>();
102+
}
88103

89104
let validation = self.client.get_validation();
90105
if validation {
@@ -150,7 +165,7 @@ impl Query {
150165
}
151166

152167
pub(crate) fn do_execute(self, read_only: bool) -> Result<Response> {
153-
let query = self.sql.finish()?;
168+
let query = self.sql.finish(self.interp_flags)?;
154169

155170
let mut url =
156171
Url::parse(&self.client.url).map_err(|err| Error::InvalidParams(Box::new(err)))?;

src/query/query_flags.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// Query interpolation flags for controlling SQL template processing.
2+
///
3+
/// This struct provides compile-time constants that control how SQL templates
4+
/// are processed, specifically for `?` parameter binding and `?fields` substitution.
5+
#[derive(Clone)]
6+
pub struct QI;
7+
8+
impl QI {
9+
/// No interpolation features enabled. `?` becomes `NULL`, `?fields` is skipped. Implemented only for test purposes
10+
pub const NONE: u8 = 0;
11+
12+
/// Enable `?fields` substitution with struct field names.
13+
pub const FIELDS: u8 = 0b0001;
14+
15+
/// Enable `?` parameter binding with `.bind()` method.
16+
pub const BIND: u8 = 0b0010;
17+
18+
/// Default flags used by `.query()` method.
19+
///
20+
/// By default, only `?fields` substitution is enabled. Parameter binding (`?`)
21+
/// is opt-in via `Client::query_with_flags`.
22+
pub const DEFAULT: u8 = QI::FIELDS;
23+
24+
/// All interpolation features enabled.
25+
pub const ALL: u8 = QI::FIELDS | QI::BIND;
26+
27+
/// Compile-time flag checking functions
28+
#[inline(always)]
29+
pub const fn has_fields(flags: u8) -> bool {
30+
(flags & Self::FIELDS) != 0
31+
}
32+
33+
#[inline(always)]
34+
pub const fn has_bind(flags: u8) -> bool {
35+
(flags & Self::BIND) != 0
36+
}
37+
}

src/sql/mod.rs

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::fmt::{self, Display, Write};
22

33
use crate::{
44
error::{Error, Result},
5+
query::QI,
56
row::{self, Row},
67
};
78

@@ -113,20 +114,39 @@ impl SqlBuilder {
113114
}
114115
}
115116

116-
pub(crate) fn finish(mut self) -> Result<String> {
117+
pub(crate) fn finish(mut self, interp_flags: u8) -> Result<String> {
117118
let mut sql = String::new();
118119

119120
if let Self::InProgress(parts, _) = &self {
120121
for part in parts {
121122
match part {
122123
Part::Text(text) => sql.push_str(text),
123124
Part::Arg => {
124-
self.error("unbound query argument");
125-
break;
125+
if QI::has_bind(interp_flags) {
126+
// Error on unbound arguments when BIND flag is set
127+
self.error("unbound query argument");
128+
break;
129+
} else {
130+
// Push NULL as placeholder when BIND flag is not set
131+
eprintln!(
132+
"warning: bind() called but QI::BIND flag not set, using NULL for query: {}",
133+
self
134+
);
135+
sql.push_str("NULL");
136+
}
126137
}
127138
Part::Fields => {
128-
self.error("unbound query argument ?fields");
129-
break;
139+
if QI::has_fields(interp_flags) {
140+
self.error("unbound query argument ?fields");
141+
break;
142+
} else {
143+
// Skip ?fields binding entirely when FIELDS flag is not set
144+
eprintln!(
145+
"warning: use QI::FIELDS template flag, ?fields skipped for query: {}",
146+
self
147+
);
148+
// Don't push anything - just skip this part
149+
}
130150
}
131151
}
132152
}
@@ -194,18 +214,35 @@ mod tests {
194214
);
195215

196216
assert_eq!(
197-
sql.finish().unwrap(),
217+
sql.finish(QI::BIND).unwrap(),
198218
r"SELECT `a`,`b` FROM test WHERE a = 'foo' AND b < 42"
199219
);
200220
}
201221

222+
#[test]
223+
fn skipped_fields() {
224+
// Test that ?fields is skipped when FIELDS flag is not set
225+
let mut sql = SqlBuilder::new("SELECT ?fields FROM test WHERE id = ?");
226+
sql.bind_arg(42);
227+
228+
// Without QI::FIELDS flag, ?fields should be skipped entirely
229+
// The bound ? becomes 42 as expected
230+
let result = sql.finish(QI::NONE).unwrap();
231+
assert_eq!(result, "SELECT FROM test WHERE id = 42");
232+
233+
// Test case with unbound ? - should become NULL when BIND flag not set
234+
let sql2 = SqlBuilder::new("SELECT ?fields FROM test WHERE id = ?");
235+
let result2 = sql2.finish(QI::NONE).unwrap();
236+
assert_eq!(result2, "SELECT FROM test WHERE id = NULL");
237+
}
238+
202239
#[test]
203240
fn in_clause() {
204241
fn t(arg: &[&str], expected: &str) {
205242
let mut sql = SqlBuilder::new("SELECT ?fields FROM test WHERE a IN ?");
206243
sql.bind_arg(arg);
207244
sql.bind_fields::<Row>();
208-
assert_eq!(sql.finish().unwrap(), expected);
245+
assert_eq!(sql.finish(QI::BIND).unwrap(), expected);
209246
}
210247

211248
const ARGS: &[&str] = &["bar", "baz", "foobar"];
@@ -228,7 +265,7 @@ mod tests {
228265
sql.bind_arg(&["a?b", "c?"][..]);
229266
sql.bind_arg("a?");
230267
assert_eq!(
231-
sql.finish().unwrap(),
268+
sql.finish(QI::BIND).unwrap(),
232269
r"SELECT 1 FROM test WHERE a IN ['a?b','c?'] AND b = 'a?'"
233270
);
234271
}
@@ -237,7 +274,7 @@ mod tests {
237274
fn question_escape() {
238275
let sql = SqlBuilder::new("SELECT 1 FROM test WHERE a IN 'a??b'");
239276
assert_eq!(
240-
sql.finish().unwrap(),
277+
sql.finish(QI::BIND).unwrap(),
241278
r"SELECT 1 FROM test WHERE a IN 'a?b'"
242279
);
243280
}
@@ -246,39 +283,51 @@ mod tests {
246283
fn option_as_null() {
247284
let mut sql = SqlBuilder::new("SELECT 1 FROM test WHERE a = ?");
248285
sql.bind_arg(None::<u32>);
249-
assert_eq!(sql.finish().unwrap(), r"SELECT 1 FROM test WHERE a = NULL");
286+
assert_eq!(
287+
sql.finish(QI::BIND).unwrap(),
288+
r"SELECT 1 FROM test WHERE a = NULL"
289+
);
250290
}
251291

252292
#[test]
253293
fn option_as_value() {
254294
let mut sql = SqlBuilder::new("SELECT 1 FROM test WHERE a = ?");
255295
sql.bind_arg(Some(1u32));
256-
assert_eq!(sql.finish().unwrap(), r"SELECT 1 FROM test WHERE a = 1");
296+
assert_eq!(
297+
sql.finish(QI::BIND).unwrap(),
298+
r"SELECT 1 FROM test WHERE a = 1"
299+
);
257300
}
258301

259302
#[test]
260303
fn failures() {
261304
let mut sql = SqlBuilder::new("SELECT 1");
262305
sql.bind_arg(42);
263-
let err = sql.finish().unwrap_err();
306+
let err = sql.finish(QI::BIND).unwrap_err();
264307
assert!(err.to_string().contains("all arguments are already bound"));
265308

266309
let mut sql = SqlBuilder::new("SELECT ?fields");
267310
sql.bind_fields::<Unnamed>();
268-
let err = sql.finish().unwrap_err();
311+
let err = sql.finish(QI::BIND | QI::FIELDS).unwrap_err();
269312
assert!(
270313
err.to_string()
271314
.contains("argument ?fields cannot be used with non-struct row types")
272315
);
273316

317+
//new changes ->
318+
// let err = sql.finish().unwrap_err();
319+
// assert!(
320+
// err.to_string()
321+
// .contains("argument ?fields cannot be used with non-struct row types")
322+
// );
274323
let mut sql = SqlBuilder::new("SELECT a FROM test WHERE b = ? AND c = ?");
275324
sql.bind_arg(42);
276-
let err = sql.finish().unwrap_err();
325+
let err = sql.finish(QI::BIND).unwrap_err();
277326
assert!(err.to_string().contains("unbound query argument"));
278327

279328
let mut sql = SqlBuilder::new("SELECT ?fields FROM test WHERE b = ?");
280329
sql.bind_arg(42);
281-
let err = sql.finish().unwrap_err();
330+
let err = sql.finish(QI::BIND | QI::FIELDS).unwrap_err();
282331
assert!(err.to_string().contains("unbound query argument ?fields"));
283332
}
284333
}

0 commit comments

Comments
 (0)