Skip to content

Commit 9c3a347

Browse files
authored
fix(prqlc, append): avoid type mismatch with postgres (#5343)
1 parent b7d8c8e commit 9c3a347

30 files changed

+1258
-109
lines changed

prqlc/prqlc/src/ir/rq/fold.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ pub fn fold_table_ref<F: ?Sized + RqFold>(fold: &mut F, table_ref: TableRef) ->
129129
Ok((fold.fold_relation_column(col)?, fold.fold_cid(cid)?))
130130
})
131131
.try_collect()?,
132+
prefer_cte: table_ref.prefer_cte,
132133
})
133134
}
134135

prqlc/prqlc/src/ir/rq/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,7 @@ pub struct TableRef {
9898

9999
/// Name hint for relation within this pipeline (table alias)
100100
pub name: Option<String>,
101+
102+
/// We prefer CTEs for most syntaxes but some like UNION works best with subqueries.
103+
pub prefer_cte: bool,
101104
}

prqlc/prqlc/src/semantic/lowering.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ impl Lowerer {
471471
source: tid,
472472
name,
473473
columns,
474+
prefer_cte: true,
474475
}
475476
}
476477

@@ -586,7 +587,8 @@ impl Lowerer {
586587
self.pipeline.push(transform);
587588
}
588589
pl::TransformKind::Append(bottom) => {
589-
let bottom = self.lower_table_ref(*bottom)?;
590+
let mut bottom = self.lower_table_ref(*bottom)?;
591+
bottom.prefer_cte = false;
590592

591593
self.pipeline.push(Transform::Append(bottom));
592594
}

prqlc/prqlc/src/semantic/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ pub mod test {
253253
- - Wildcard
254254
- 0
255255
name: employees
256+
prefer_cte: true
256257
- Select:
257258
- 0
258259
columns:

prqlc/prqlc/src/semantic/resolver/transforms.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,7 @@ mod tests {
11331133
- - Wildcard
11341134
- 1
11351135
name: c_invoice
1136+
prefer_cte: true
11361137
- Select:
11371138
- 0
11381139
- Take:
@@ -1227,6 +1228,7 @@ mod tests {
12271228
- - Wildcard
12281229
- 3
12291230
name: invoices
1231+
prefer_cte: true
12301232
- Sort:
12311233
- direction: Asc
12321234
column: 0

prqlc/prqlc/src/sql/dialect.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ pub(super) trait DialectHandler: Any + Debug {
213213
named: false,
214214
}))
215215
}
216+
217+
/// Whether source and subqueries should be put between simple parentheses
218+
/// for `UNION` and similar verbs.
219+
fn prefers_subquery_parentheses_shorthand(&self) -> bool {
220+
false
221+
}
216222
}
217223

218224
impl dyn DialectHandler {
@@ -282,6 +288,10 @@ impl DialectHandler for PostgresDialect {
282288
fn supports_zero_columns(&self) -> bool {
283289
true
284290
}
291+
292+
fn prefers_subquery_parentheses_shorthand(&self) -> bool {
293+
true
294+
}
285295
}
286296

287297
impl DialectHandler for GlareDbDialect {
@@ -474,6 +484,10 @@ impl DialectHandler for BigQueryDialect {
474484
// https://cloud.google.com/bigquery/docs/reference/standard-sql/query-syntax#set_operators
475485
true
476486
}
487+
488+
fn prefers_subquery_parentheses_shorthand(&self) -> bool {
489+
true
490+
}
477491
}
478492

479493
impl DialectHandler for SnowflakeDialect {

prqlc/prqlc/src/sql/gen_query.rs

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -265,21 +265,37 @@ fn translate_set_ops_pipeline(
265265
_ => unreachable!(),
266266
};
267267

268-
// prepare top
268+
// Some engines (like SQLite) do not support subqueries between simple parentheses, so we keep
269+
// the general `SELECT * FROM (query)` or `SELECT * FROM cte` by default for complex cases.
270+
//
271+
// Some engines (like Postgres) need the subquery directly to properly match column type on
272+
// both sides. For those:
273+
// 1. we need the subquery as-is or in parentheses
274+
// 2. we need to avoid CTEs
275+
276+
// Left hand side (aka top)
269277
let left = query_to_set_expr(top, context);
270278

271-
top = default_query(SetExpr::SetOperation {
272-
left,
273-
right: Box::new(SetExpr::Select(Box::new(sql_ast::Select {
279+
// Right hand side (aka bottom)
280+
let right_rel = translate_relation_expr(bottom, context)?;
281+
let right = if let TableFactor::Derived { subquery, .. } = right_rel {
282+
query_to_set_expr(*subquery, context)
283+
} else {
284+
Box::new(SetExpr::Select(Box::new(sql_ast::Select {
274285
projection: vec![SelectItem::Wildcard(
275286
sql_ast::WildcardAdditionalOptions::default(),
276287
)],
277288
from: vec![TableWithJoins {
278-
relation: translate_relation_expr(bottom, context)?,
289+
relation: right_rel,
279290
joins: vec![],
280291
}],
281292
..default_select()
282-
}))),
293+
})))
294+
};
295+
296+
top = default_query(SetExpr::SetOperation {
297+
left,
298+
right,
283299
set_quantifier: if distinct {
284300
if context.dialect.set_ops_distinct() {
285301
sql_ast::SetQuantifier::Distinct
@@ -635,24 +651,36 @@ fn query_to_set_expr(query: sql_ast::Query, context: &mut Context) -> Box<SetExp
635651
return query.body;
636652
}
637653

638-
// query is not simple, so we need to wrap it into
639-
// `SELECT * FROM (query)`
640-
Box::new(SetExpr::Select(Box::new(Select {
641-
projection: vec![SelectItem::Wildcard(
642-
sql_ast::WildcardAdditionalOptions::default(),
643-
)],
644-
from: vec![TableWithJoins {
645-
relation: TableFactor::Derived {
646-
lateral: false,
647-
subquery: Box::new(query),
648-
alias: Some(simple_table_alias(sql_ast::Ident::new(
649-
context.anchor.table_name.gen(),
650-
))),
651-
},
652-
joins: vec![],
653-
}],
654-
..default_select()
655-
})))
654+
// Query is not simple, so we need to wrap it.
655+
//
656+
// Some engines (like SQLite) do not support subqueries between simple parentheses, so we keep
657+
// the general `SELECT * FROM (query)` by default.
658+
//
659+
// Some engines (like Postgres) may need the subquery directly as-is or between parentheses
660+
// to properly match columns for syntaxes like `UNION`, `EXCEPT`, and `INTERSECT`.
661+
// Incidentally, the parenthesis syntax `(query)` allows for complex queries.
662+
let set_expr = if context.dialect.prefers_subquery_parentheses_shorthand() {
663+
SetExpr::Query(query.into())
664+
} else {
665+
SetExpr::Select(Box::new(Select {
666+
projection: vec![SelectItem::Wildcard(
667+
sql_ast::WildcardAdditionalOptions::default(),
668+
)],
669+
from: vec![TableWithJoins {
670+
relation: TableFactor::Derived {
671+
lateral: false,
672+
subquery: Box::new(query),
673+
alias: Some(simple_table_alias(sql_ast::Ident::new(
674+
context.anchor.table_name.gen(),
675+
))),
676+
},
677+
joins: vec![],
678+
}],
679+
..default_select()
680+
}))
681+
};
682+
683+
Box::new(set_expr)
656684
}
657685

658686
fn count_tables(transforms: &[Transform]) -> usize {

prqlc/prqlc/src/sql/pq/anchor.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ pub(super) fn anchor_split(
291291
source: new_tid,
292292
name: None,
293293
columns: new_columns,
294+
prefer_cte: true,
294295
},
295296
cid_redirects,
296297
);

prqlc/prqlc/src/sql/pq/gen_query.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ pub(super) fn compile_relation_instance(riid: RIId, ctx: &mut Context) -> Result
189189
// ensure that the table is declared
190190
if let RelationStatus::NotYetDefined(sql_relation) = decl.relation.take_to_define() {
191191
// if we cannot use CTEs (probably because we are within RECURSIVE)
192-
if !ctx.query.allow_ctes {
192+
if !(ctx.query.allow_ctes && table_ref.prefer_cte) {
193193
// restore relation for other references
194194
decl.relation = RelationStatus::NotYetDefined(sql_relation.clone());
195195

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
from invoices
2+
select {an_id = invoice_id, name = null}
3+
take 2
4+
append (
5+
from employees
6+
select {an_id = null, name = first_name}
7+
take 2
8+
)

0 commit comments

Comments
 (0)