Skip to content

Commit c64c07a

Browse files
rs-sacrs-sac
committed
Plumb explicit dialect through to various display sites
We assumed MySQL because we were lazy. Fixes: REA-2168 Change-Id: I82ec27617b08777cae2a28723677dfc6ea1309f1 Reviewed-on: https://gerrit.readyset.name/c/readyset/+/10110 Reviewed-by: Jason Brown <jason.b@readyset.io> Tested-by: Buildkite CI
1 parent 64b86a4 commit c64c07a

File tree

20 files changed

+235
-198
lines changed

20 files changed

+235
-198
lines changed

‎dataflow-expression/src/lower.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1468,7 +1468,7 @@ impl Expr {
14681468
| AstExpr::WindowFunction { .. } => {
14691469
internal!(
14701470
"Expression should have been desugared earlier: {}",
1471-
expr.display(readyset_sql::Dialect::MySQL)
1471+
expr.display(dialect.into())
14721472
)
14731473
}
14741474
}

‎readyset-adapter/src/migration_handler.rs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,7 @@ impl MigrationHandler {
134134
Err(e) if e.caused_by_unsupported() => {
135135
debug!(
136136
error = %e,
137-
// FIXME(REA-2169): Use correct dialect.
138-
query = %Sensitive(&query.query().statement.display(readyset_sql::Dialect::MySQL)),
137+
query = %Sensitive(&query.query().statement.display(self.dialect.into())),
139138
"Select query is unsupported in ReadySet"
140139
);
141140
self.query_status_cache
@@ -147,8 +146,7 @@ impl MigrationHandler {
147146
Err(e) => {
148147
debug!(
149148
error = %e,
150-
// FIXME(REA-2169): Use correct dialect.
151-
query = %Sensitive(&query.query().statement.display(readyset_sql::Dialect::MySQL)),
149+
query = %Sensitive(&query.query().statement.display(self.dialect.into())),
152150
"Select query may have transiently failed"
153151
);
154152
if Instant::now() - *self.start_time.get(query.query()).unwrap()
@@ -242,8 +240,7 @@ impl MigrationHandler {
242240
Err(e) if e.caused_by_unsupported() => {
243241
debug!(
244242
error = %e,
245-
// FIXME(REA-2168 + REA-2169): Use correct dialect.
246-
query = %Sensitive(&view_request.statement.display(readyset_sql::Dialect::MySQL)),
243+
query = %Sensitive(&view_request.statement.display(self.dialect.into())),
247244
"Select query is unsupported in ReadySet"
248245
);
249246

@@ -258,8 +255,7 @@ impl MigrationHandler {
258255
Err(e) => {
259256
debug!(
260257
error = %e,
261-
// FIXME(REA-2168 + REA-2169): Use correct dialect.
262-
query = %Sensitive(&view_request.statement.display(readyset_sql::Dialect::MySQL)),
258+
query = %Sensitive(&view_request.statement.display(self.dialect.into())),
263259
"Select query may have transiently failed"
264260
);
265261
if Instant::now() - *self.start_time.get(view_request).unwrap() > self.max_retry {

‎readyset-adapter/src/views_synchronizer.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,7 @@ impl ViewsSynchronizer {
173173
let chunk_hashes = hashes.iter().take(chunk.len());
174174
for ((query, name), hash) in chunk.iter().zip(statuses).zip(chunk_hashes) {
175175
trace!(
176-
// FIXME(REA-2168): Use correct dialect.
177-
query = %query.statement.display(readyset_sql::Dialect::MySQL),
176+
query = %query.statement.display(self.dialect.into()),
178177
name = ?name,
179178
"Loaded query status from controller"
180179
);

‎readyset-logictest/src/from_query_log.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,7 @@ impl FromQueryLog {
184184
.map(Value::try_from)
185185
.collect::<Result<Vec<_>, _>>()?;
186186

187-
// FIXME(REA-2168): Use correct dialect.
188-
let stmt_string = stmt.display(readyset_sql::Dialect::MySQL).to_string();
187+
let stmt_string = stmt.display(self.database.dialect()).to_string();
189188

190189
let rows = conn.execute(&stmt_string, &params).await?;
191190

‎readyset-mir/src/rewrite/filters_to_join_keys.rs‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,10 @@ impl FunctionProcessor<'_> {
137137
/// ```
138138
///
139139
/// Also works with functions over join columns.
140-
pub(crate) fn convert_filters_to_join_keys(query: &mut MirQuery<'_>) -> ReadySetResult<()> {
141-
let dialect = Dialect::MySQL;
142-
140+
pub(crate) fn convert_filters_to_join_keys(
141+
query: &mut MirQuery<'_>,
142+
dialect: Dialect,
143+
) -> ReadySetResult<()> {
143144
// We'll be constructing a map from join_idx -> Vec<(filter_idx, (left_join_col,
144145
// right_join_col))>
145146
let mut filters_to_add = BTreeMap::<_, Vec<_>>::new();
@@ -524,7 +525,7 @@ mod tests {
524525

525526
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
526527

527-
convert_filters_to_join_keys(&mut query).unwrap();
528+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
528529
assert!(!mir_graph.contains_node(filter));
529530
match &mir_graph[join].inner {
530531
MirNodeInner::Join { on, .. } => assert_eq!(
@@ -573,7 +574,7 @@ mod tests {
573574

574575
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
575576

576-
convert_filters_to_join_keys(&mut query).unwrap();
577+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
577578
assert!(!mir_graph.contains_node(filter));
578579
match &mir_graph[join].inner {
579580
MirNodeInner::Join { on, .. } => assert_eq!(
@@ -615,7 +616,7 @@ mod tests {
615616

616617
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
617618

618-
convert_filters_to_join_keys(&mut query).unwrap();
619+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
619620

620621
assert!(mir_graph.contains_node(filter), "Filter is not a join key!");
621622
}
@@ -658,7 +659,7 @@ mod tests {
658659

659660
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
660661

661-
convert_filters_to_join_keys(&mut query).unwrap();
662+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
662663

663664
assert!(mir_graph.contains_node(filter), "Filter is not a join key!");
664665
}
@@ -688,7 +689,7 @@ mod tests {
688689

689690
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
690691

691-
convert_filters_to_join_keys(&mut query).unwrap();
692+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
692693

693694
assert!(!mir_graph.contains_node(filter));
694695
match &mir_graph[join].inner {
@@ -725,7 +726,7 @@ mod tests {
725726

726727
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
727728

728-
convert_filters_to_join_keys(&mut query).unwrap();
729+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
729730

730731
assert!(!mir_graph.contains_node(filter));
731732
match &mir_graph[join].inner {
@@ -762,7 +763,7 @@ mod tests {
762763

763764
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
764765

765-
convert_filters_to_join_keys(&mut query).unwrap();
766+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
766767

767768
assert!(!mir_graph.contains_node(filter));
768769
match &mir_graph[join].inner {
@@ -801,7 +802,7 @@ mod tests {
801802

802803
let mut query = MirQuery::new(query_name, leaf, &mut mir_graph);
803804

804-
convert_filters_to_join_keys(&mut query).unwrap();
805+
convert_filters_to_join_keys(&mut query, readyset_sql::Dialect::MySQL).unwrap();
805806

806807
assert!(!mir_graph.contains_node(filter));
807808
match &mir_graph[join].inner {

‎readyset-mir/src/rewrite/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ mod pull_keys;
1313
impl MirQuery<'_> {
1414
/// Run a set of rewrite and optimization passes on this [`MirQuery`], and returns the modified
1515
/// query.
16-
pub fn rewrite(mut self) -> ReadySetResult<Self> {
16+
pub fn rewrite(mut self, dialect: readyset_sql::Dialect) -> ReadySetResult<Self> {
1717
pull_keys::pull_view_keys_to_leaf(&mut self)?;
1818
decorrelate::eliminate_dependent_joins(&mut self)?;
1919
predicate_pushup::push_filters_up(&mut self)?;
20-
filters_to_join_keys::convert_filters_to_join_keys(&mut self)?;
20+
filters_to_join_keys::convert_filters_to_join_keys(&mut self, dialect)?;
2121
add_bogokey::add_bogokey_if_necessary(&mut self)?;
2222
pull_columns::pull_all_required_columns(&mut self)?;
2323
fuse::fuse_project_nodes(&mut self)?;

‎readyset-server/src/controller/sql/mir/grouped.rs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,9 @@ pub(super) fn make_expressions_above_grouped(
7575
.cloned()
7676
.map(|expr| {
7777
let x = (
78-
// FIXME(ENG-2502): Use correct dialect.
7978
SqlIdentifier::from(
8079
expr.clone()
81-
.display(readyset_sql::Dialect::MySQL)
80+
.display(mir_converter.dialect.into())
8281
.to_string(),
8382
),
8483
expr,
@@ -101,10 +100,9 @@ pub(super) fn make_expressions_above_grouped(
101100
None
102101
}
103102
expr => Some((
104-
// FIXME(ENG-2502): Use correct dialect.
105103
SqlIdentifier::from(
106104
expr.clone()
107-
.display(readyset_sql::Dialect::MySQL)
105+
.display(mir_converter.dialect.into())
108106
.to_string(),
109107
),
110108
expr.clone(),
@@ -184,7 +182,7 @@ pub(super) fn make_grouped(
184182
Expr::Column(c) => c.clone(),
185183
expr => ast::Column {
186184
name: expr
187-
.display(readyset_sql::Dialect::MySQL)
185+
.display(mir_converter.dialect.into())
188186
.to_string()
189187
.into(),
190188
table: None,
@@ -304,6 +302,7 @@ fn joinable_aggregate_nodes(
304302
pub(super) fn post_lookup_aggregates(
305303
query_graph: &QueryGraph,
306304
query_name: &Relation,
305+
dialect: readyset_data::Dialect,
307306
) -> ReadySetResult<Option<PostLookupAggregates<Column>>> {
308307
if query_graph.distinct {
309308
// DISTINCT is the equivalent of grouping by all projected columns but not actually doing
@@ -321,8 +320,7 @@ pub(super) fn post_lookup_aggregates(
321320
..
322321
} => Some(Column::from(col).aliased_as_table(query_name.clone())),
323322
FieldDefinitionExpr::Expr { expr, .. } => Some(
324-
// FIXME(ENG-2502): Use correct dialect.
325-
Column::named(expr.display(readyset_sql::Dialect::MySQL).to_string())
323+
Column::named(expr.display(dialect.into()).to_string())
326324
.aliased_as_table(query_name.clone()),
327325
),
328326
_ => None,
@@ -384,7 +382,7 @@ pub(super) fn post_lookup_aggregates(
384382
.map(|c| {
385383
match c {
386384
Expr::Column(c) => c.clone().into(),
387-
expr => Column::named(expr.display(readyset_sql::Dialect::MySQL).to_string()),
385+
expr => Column::named(expr.display(dialect.into()).to_string()),
388386
}
389387
.aliased_as_table(query_name.clone())
390388
})

0 commit comments

Comments
 (0)