Skip to content

Commit ae29fa0

Browse files
georgestagggeorgestagg
committed
ODBC: Quoting is fun
1 parent f6e2b4e commit ae29fa0

12 files changed

Lines changed: 83 additions & 92 deletions

File tree

‎src/execute/casting.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ pub fn determine_layer_source(
216216
match &layer.source {
217217
Some(DataSource::Identifier(name)) => {
218218
if materialized_ctes.contains(name) {
219-
naming::quote_ident(&naming::cte_table(name))
219+
format!("\"{}\"", naming::cte_table(name))
220220
} else {
221221
name.clone()
222222
}
@@ -227,7 +227,7 @@ pub fn determine_layer_source(
227227
None => {
228228
// Layer uses global data - caller must ensure has_global is true
229229
debug_assert!(has_global, "Layer has no source and no global data");
230-
naming::quote_ident(&naming::global_table())
230+
format!("\"{}\"", naming::global_table())
231231
}
232232
}
233233
}

‎src/execute/cte.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ pub fn transform_cte_references(sql: &str, cte_names: &HashSet<String>) -> Strin
9494
let mut result = sql.to_string();
9595

9696
for cte_name in cte_names {
97-
let temp_table_name = naming::quote_ident(&naming::cte_table(cte_name));
97+
let temp_table_name = format!("\"{}\"", naming::cte_table(cte_name));
9898

9999
// Replace table references: FROM cte_name, JOIN cte_name, cte_name.column
100100
// Use word boundary matching to avoid replacing substrings

‎src/execute/layer.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ pub fn apply_pre_stat_transform(
271271
.collect();
272272

273273
format!(
274-
"SELECT {} FROM ({}) AS __ggsql_pre__",
274+
"SELECT {} FROM ({}) AS \"__ggsql_pre__\"",
275275
select_exprs.join(", "),
276276
query
277277
)
@@ -315,14 +315,14 @@ pub fn build_layer_base_query(
315315
// Build query with optional WHERE clause
316316
if let Some(ref f) = layer.filter {
317317
format!(
318-
"SELECT {} FROM ({}) AS __ggsql_src__ WHERE {}",
318+
"SELECT {} FROM ({}) AS \"__ggsql_src__\" WHERE {}",
319319
select_clause,
320320
source_query,
321321
f.as_str()
322322
)
323323
} else {
324324
format!(
325-
"SELECT {} FROM ({}) AS __ggsql_src__",
325+
"SELECT {} FROM ({}) AS \"__ggsql_src__\"",
326326
select_clause, source_query
327327
)
328328
}
@@ -542,14 +542,14 @@ where
542542
.and_then(super::cte::split_with_query)
543543
{
544544
format!(
545-
"{}, __ggsql_stat__ AS ({}) SELECT *, {} FROM __ggsql_stat__",
545+
"{}, \"__ggsql_stat__\" AS ({}) SELECT *, {} FROM \"__ggsql_stat__\"",
546546
cte_prefix,
547547
trailing_select,
548548
stat_rename_exprs.join(", ")
549549
)
550550
} else {
551551
format!(
552-
"SELECT *, {} FROM ({}) AS __ggsql_stat__",
552+
"SELECT *, {} FROM ({}) AS \"__ggsql_stat__\"",
553553
stat_rename_exprs.join(", "),
554554
transformed_query
555555
)

‎src/execute/schema.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub fn build_minmax_query(source_query: &str, column_names: &[&str]) -> String {
3030
.collect();
3131

3232
format!(
33-
"WITH __ggsql_source__ AS ({}) SELECT {} FROM __ggsql_source__ UNION ALL SELECT {} FROM __ggsql_source__",
33+
"WITH \"__ggsql_source__\" AS ({}) SELECT {} FROM \"__ggsql_source__\" UNION ALL SELECT {} FROM \"__ggsql_source__\"",
3434
source_query,
3535
min_exprs.join(", "),
3636
max_exprs.join(", ")

‎src/naming.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,6 @@ pub const SOURCE_COLUMN: &str = concatcp!(GGSQL_PREFIX, "source", GGSQL_SUFFIX);
8888
/// Alias for schema extraction queries
8989
pub const SCHEMA_ALIAS: &str = concatcp!(GGSQL_SUFFIX, "schema", GGSQL_SUFFIX);
9090

91-
// ============================================================================
92-
// Quoting
93-
// ============================================================================
94-
95-
/// Quote a SQL identifier with double quotes (ANSI SQL).
96-
///
97-
/// This ensures case-preserving behavior on backends like Snowflake that
98-
/// uppercase unquoted identifiers.
99-
pub fn quote_ident(name: &str) -> String {
100-
format!("\"{}\"", name.replace('"', "\"\""))
101-
}
102-
10391
// ============================================================================
10492
// Constructor Functions
10593
// ============================================================================

‎src/plot/layer/geom/bar.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,19 +169,19 @@ fn stat_bar_count(
169169
if let Some(weight_col) = weight_value.column_name() {
170170
if schema_columns.contains(weight_col) {
171171
// weight column exists - use SUM (but still call it "count")
172-
format!("SUM({}) AS {}", weight_col, stat_count)
172+
format!("SUM({}) AS \"{}\"", weight_col, stat_count)
173173
} else {
174174
// weight mapped but column doesn't exist - fall back to COUNT
175175
// (this shouldn't happen with upfront validation, but handle gracefully)
176-
format!("COUNT(*) AS {}", stat_count)
176+
format!("COUNT(*) AS \"{}\"", stat_count)
177177
}
178178
} else {
179179
// Shouldn't happen (not literal, not column), fall back to COUNT
180-
format!("COUNT(*) AS {}", stat_count)
180+
format!("COUNT(*) AS \"{}\"", stat_count)
181181
}
182182
} else {
183183
// weight not mapped - use COUNT
184-
format!("COUNT(*) AS {}", stat_count)
184+
format!("COUNT(*) AS \"{}\"", stat_count)
185185
};
186186

187187
// Build the query based on whether x is mapped or not
@@ -191,13 +191,13 @@ fn stat_bar_count(
191191
let (grouped_select, final_select) = if group_by.is_empty() {
192192
(
193193
format!(
194-
"'{dummy}' AS {x}, {agg}",
194+
"'{dummy}' AS \"{x}\", {agg}",
195195
dummy = stat_dummy_value,
196196
x = stat_x,
197197
agg = agg_expr
198198
),
199199
format!(
200-
"*, {count} * 1.0 / SUM({count}) OVER () AS {prop}",
200+
"*, \"{count}\" * 1.0 / SUM(\"{count}\") OVER () AS \"{prop}\"",
201201
count = stat_count,
202202
prop = stat_proportion
203203
),
@@ -206,14 +206,14 @@ fn stat_bar_count(
206206
let grp_cols = group_by.join(", ");
207207
(
208208
format!(
209-
"{g}, '{dummy}' AS {x}, {agg}",
209+
"{g}, '{dummy}' AS \"{x}\", {agg}",
210210
g = grp_cols,
211211
dummy = stat_dummy_value,
212212
x = stat_x,
213213
agg = agg_expr
214214
),
215215
format!(
216-
"*, {count} * 1.0 / SUM({count}) OVER (PARTITION BY {grp}) AS {prop}",
216+
"*, \"{count}\" * 1.0 / SUM(\"{count}\") OVER (PARTITION BY {grp}) AS \"{prop}\"",
217217
count = stat_count,
218218
grp = grp_cols,
219219
prop = stat_proportion
@@ -224,7 +224,7 @@ fn stat_bar_count(
224224
let query_str = if group_by.is_empty() {
225225
// No grouping at all - single aggregate
226226
format!(
227-
"WITH __stat_src__ AS ({query}), __grouped__ AS (SELECT {grouped} FROM __stat_src__) SELECT {final} FROM __grouped__",
227+
"WITH \"__stat_src__\" AS ({query}), \"__grouped__\" AS (SELECT {grouped} FROM \"__stat_src__\") SELECT {final} FROM \"__grouped__\"",
228228
query = query,
229229
grouped = grouped_select,
230230
final = final_select
@@ -233,7 +233,7 @@ fn stat_bar_count(
233233
// Group by partition/facet variables only
234234
let group_cols = group_by.join(", ");
235235
format!(
236-
"WITH __stat_src__ AS ({query}), __grouped__ AS (SELECT {grouped} FROM __stat_src__ GROUP BY {group}) SELECT {final} FROM __grouped__",
236+
"WITH \"__stat_src__\" AS ({query}), \"__grouped__\" AS (SELECT {grouped} FROM \"__stat_src__\" GROUP BY {group}) SELECT {final} FROM \"__grouped__\"",
237237
query = query,
238238
grouped = grouped_select,
239239
group = group_cols,
@@ -271,7 +271,7 @@ fn stat_bar_count(
271271
(
272272
format!("{x}, {agg}", x = x_col, agg = agg_expr),
273273
format!(
274-
"*, {count} * 1.0 / SUM({count}) OVER () AS {prop}",
274+
"*, \"{count}\" * 1.0 / SUM(\"{count}\") OVER () AS \"{prop}\"",
275275
count = stat_count,
276276
prop = stat_proportion
277277
),
@@ -281,7 +281,7 @@ fn stat_bar_count(
281281
(
282282
format!("{g}, {x}, {agg}", g = grp_cols, x = x_col, agg = agg_expr),
283283
format!(
284-
"*, {count} * 1.0 / SUM({count}) OVER (PARTITION BY {grp}) AS {prop}",
284+
"*, \"{count}\" * 1.0 / SUM(\"{count}\") OVER (PARTITION BY {grp}) AS \"{prop}\"",
285285
count = stat_count,
286286
grp = grp_cols,
287287
prop = stat_proportion
@@ -290,7 +290,7 @@ fn stat_bar_count(
290290
};
291291

292292
let query_str = format!(
293-
"WITH __stat_src__ AS ({query}), __grouped__ AS (SELECT {grouped} FROM __stat_src__ GROUP BY {group}) SELECT {final} FROM __grouped__",
293+
"WITH \"__stat_src__\" AS ({query}), \"__grouped__\" AS (SELECT {grouped} FROM \"__stat_src__\" GROUP BY {group}) SELECT {final} FROM \"__grouped__\"",
294294
query = query,
295295
grouped = grouped_select,
296296
group = group_cols,

‎src/plot/layer/geom/boxplot.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ fn boxplot_sql_compute_summary(
177177
let q1 = dialect.sql_percentile(value, 0.25, from, groups);
178178
let median = dialect.sql_percentile(value, 0.50, from, groups);
179179
let q3 = dialect.sql_percentile(value, 0.75, from, groups);
180+
let qt = "\"__ggsql_qt__\"";
181+
let fn_alias = "\"__ggsql_fn__\"";
180182
format!(
181183
"SELECT
182184
*,
@@ -190,10 +192,10 @@ fn boxplot_sql_compute_summary(
190192
{q1} AS q1,
191193
{median} AS median,
192194
{q3} AS q3
193-
FROM ({from}) AS __ggsql_qt__
195+
FROM ({from}) AS {qt}
194196
WHERE {value} IS NOT NULL
195197
GROUP BY {groups}
196-
) AS __ggsql_fn__",
198+
) AS {fn_alias}",
197199
lower_expr = lower_expr,
198200
upper_expr = upper_expr,
199201
groups = groups_str,
@@ -397,10 +399,10 @@ mod tests {
397399
{q1} AS q1,
398400
{median} AS median,
399401
{q3} AS q3
400-
FROM (SELECT * FROM sales) AS __ggsql_qt__
402+
FROM (SELECT * FROM sales) AS "__ggsql_qt__"
401403
WHERE price IS NOT NULL
402404
GROUP BY category
403-
) AS __ggsql_fn__"#
405+
) AS "__ggsql_fn__""#
404406
);
405407

406408
assert_eq!(result, expected);
@@ -433,10 +435,10 @@ mod tests {
433435
{q1} AS q1,
434436
{median} AS median,
435437
{q3} AS q3
436-
FROM (SELECT * FROM data) AS __ggsql_qt__
438+
FROM (SELECT * FROM data) AS "__ggsql_qt__"
437439
WHERE revenue IS NOT NULL
438440
GROUP BY region, product
439-
) AS __ggsql_fn__"#
441+
) AS "__ggsql_fn__""#
440442
);
441443

442444
assert_eq!(result, expected);

0 commit comments

Comments
 (0)