Skip to content

Commit b5344cf

Browse files
MohamedAbdeen-rsMohamedAbdeen-rs
committed
readyset-sql-parsing: Wrap select stmt parser
Testing to see if there's any mismatch between nom-sql and sqlparser when it comes to select stmts. Change-Id: I54129dfb68c4dd6baa77de5d7c423c58e323a976 Reviewed-on: https://gerrit.readyset.name/c/readyset/+/9202 Tested-by: Buildkite CI Reviewed-by: Michael Zink <[email protected]>
1 parent 995507e commit b5344cf

File tree

10 files changed

+141
-99
lines changed

10 files changed

+141
-99
lines changed

‎readyset-server/src/controller/mod.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -1357,7 +1357,7 @@ mod tests {
13571357
use std::collections::BTreeMap;
13581358

13591359
use dataflow::DomainIndex;
1360-
use nom_sql::{parse_create_table, parse_select_statement};
1360+
use nom_sql::parse_create_table;
13611361
use readyset_client::debug::info::KeyCount;
13621362
use readyset_client::recipe::changelist::{Change, ChangeList};
13631363
use readyset_client::{
@@ -1366,6 +1366,7 @@ mod tests {
13661366
use readyset_data::Dialect as DataDialect;
13671367
use readyset_sql::ast::{NonReplicatedRelation, NotReplicatedReason, Relation};
13681368
use readyset_sql::Dialect;
1369+
use readyset_sql_parsing::parse_select;
13691370
use readyset_util::eventually;
13701371
use replication_offset::ReplicationOffset;
13711372
use replicators::MySqlPosition;
@@ -1551,8 +1552,8 @@ mod tests {
15511552
async fn view_names() {
15521553
let (mut noria, shutdown_tx) = start_simple("view_names").await;
15531554

1554-
let query1 = parse_select_statement(Dialect::MySQL, "SELECT * FROM t1").unwrap();
1555-
let query2 = parse_select_statement(Dialect::MySQL, "SELECT * FROM t2").unwrap();
1555+
let query1 = parse_select(Dialect::MySQL, "SELECT * FROM t1").unwrap();
1556+
let query2 = parse_select(Dialect::MySQL, "SELECT * FROM t2").unwrap();
15561557

15571558
let schema_search_path = vec!["s1".into()];
15581559

@@ -1596,7 +1597,7 @@ mod tests {
15961597
assert!(matches!(&res2[..], [Some(_), None]));
15971598

15981599
// A syntactically distinct, but semantically equivalent query
1599-
let query1_equivalent = parse_select_statement(Dialect::MySQL, "SELECT x FROM t1").unwrap();
1600+
let query1_equivalent = parse_select(Dialect::MySQL, "SELECT x FROM t1").unwrap();
16001601
let res3 = noria
16011602
.view_names(
16021603
vec![ViewCreateRequest::new(
@@ -1610,7 +1611,7 @@ mod tests {
16101611
assert!(matches!(&res3[..], &[Some(_)]));
16111612

16121613
// A change in schema_search_path that doesn't change the semantics
1613-
let query1_equivalent = parse_select_statement(Dialect::MySQL, "SELECT x FROM t1").unwrap();
1614+
let query1_equivalent = parse_select(Dialect::MySQL, "SELECT x FROM t1").unwrap();
16141615
let res3 = noria
16151616
.view_names(
16161617
vec![ViewCreateRequest::new(
@@ -1635,7 +1636,7 @@ mod tests {
16351636
.await
16361637
.unwrap();
16371638

1638-
let query1_equivalent = parse_select_statement(Dialect::MySQL, "SELECT x FROM t1").unwrap();
1639+
let query1_equivalent = parse_select(Dialect::MySQL, "SELECT x FROM t1").unwrap();
16391640
let res3 = noria
16401641
.view_names(
16411642
vec![ViewCreateRequest::new(

‎readyset-server/src/controller/sql/query_graph.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1528,10 +1528,10 @@ pub fn to_query_graph(stmt: SelectStatement) -> ReadySetResult<QueryGraph> {
15281528
#[allow(clippy::unwrap_used)]
15291529
#[cfg(test)]
15301530
mod tests {
1531-
use nom_sql::parse_select_statement;
15321531
use readyset_sql::ast::{FunctionExpr, SqlQuery};
15331532
use readyset_sql::Dialect;
15341533
use readyset_sql_parsing::parse_query;
1534+
use readyset_sql_parsing::parse_select;
15351535

15361536
use super::*;
15371537

@@ -1687,7 +1687,7 @@ mod tests {
16871687

16881688
#[test]
16891689
fn duplicate_subquery_name() {
1690-
let query = parse_select_statement(
1690+
let query = parse_select(
16911691
Dialect::MySQL,
16921692
"select * from (select * from t) q1, (select * from t) q1;",
16931693
)

‎readyset-server/src/controller/sql/registry.rs

+23-29
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ mod tests {
698698
use super::*;
699699

700700
fn recipe_expr_cache(name: &'static str, query: &'static str) -> RecipeExpr {
701-
let statement = nom_sql::parse_select_statement(Dialect::MySQL, query).unwrap();
701+
let statement = readyset_sql_parsing::parse_select(Dialect::MySQL, query).unwrap();
702702

703703
RecipeExpr::Cache {
704704
name: name.into(),
@@ -712,14 +712,15 @@ mod tests {
712712
RecipeExpr::View {
713713
name: name.into(),
714714
definition: SelectSpecification::Simple(
715-
nom_sql::parse_select_statement(Dialect::MySQL, query).unwrap(),
715+
readyset_sql_parsing::parse_select(Dialect::MySQL, query).unwrap(),
716716
),
717717
}
718718
}
719719

720720
mod expression {
721-
use nom_sql::{parse_create_table, parse_select_statement};
721+
use nom_sql::parse_create_table;
722722
use readyset_sql::Dialect;
723+
use readyset_sql_parsing::parse_select;
723724

724725
use super::*;
725726

@@ -742,7 +743,7 @@ mod tests {
742743
let view = RecipeExpr::View {
743744
name: view_name.clone(),
744745
definition: SelectSpecification::Simple(
745-
parse_select_statement(Dialect::MySQL, "SELECT * FROM test_table;").unwrap(),
746+
parse_select(Dialect::MySQL, "SELECT * FROM test_table;").unwrap(),
746747
),
747748
};
748749

@@ -767,7 +768,7 @@ mod tests {
767768
let view = RecipeExpr::View {
768769
name: "test_view".into(),
769770
definition: SelectSpecification::Simple(
770-
parse_select_statement(Dialect::MySQL, "SELECT * FROM test_table;").unwrap(),
771+
parse_select(Dialect::MySQL, "SELECT * FROM test_table;").unwrap(),
771772
),
772773
};
773774

@@ -778,8 +779,9 @@ mod tests {
778779
}
779780

780781
mod registry {
781-
use nom_sql::{parse_create_table, parse_select_statement};
782+
use nom_sql::parse_create_table;
782783
use readyset_sql::Dialect;
784+
use readyset_sql_parsing::parse_select;
783785

784786
use super::*;
785787

@@ -851,8 +853,7 @@ mod tests {
851853
let view = RecipeExpr::View {
852854
name: view_name.clone(),
853855
definition: SelectSpecification::Simple(
854-
parse_select_statement(Dialect::MySQL, "SELECT DISTINCT * FROM test_table;")
855-
.unwrap(),
856+
parse_select(Dialect::MySQL, "SELECT DISTINCT * FROM test_table;").unwrap(),
856857
),
857858
};
858859
let num_expressions = registry.expressions.len();
@@ -881,8 +882,7 @@ mod tests {
881882
#[ignore]
882883
fn add_existing_view() {
883884
let mut registry = setup();
884-
let select =
885-
parse_select_statement(Dialect::MySQL, "SELECT * FROM test_table;").unwrap();
885+
let select = parse_select(Dialect::MySQL, "SELECT * FROM test_table;").unwrap();
886886
let view_name: Relation = "test_view2".into();
887887
let view = RecipeExpr::View {
888888
name: view_name.clone(),
@@ -1013,7 +1013,7 @@ mod tests {
10131013
#[test]
10141014
fn contains() {
10151015
let mut registry = setup();
1016-
let stmt = parse_select_statement(Dialect::MySQL, "SELECT * FROM test_table").unwrap();
1016+
let stmt = parse_select(Dialect::MySQL, "SELECT * FROM test_table").unwrap();
10171017
let expr = recipe_expr_cache("test", "SELECT * FROM test_table");
10181018

10191019
registry.add_query(expr).unwrap();
@@ -1121,8 +1121,7 @@ mod tests {
11211121
registry.add_custom_type(ty.clone());
11221122

11231123
let statement =
1124-
parse_select_statement(Dialect::PostgreSQL, "SELECT CAST(x AS public.abc) FROM t")
1125-
.unwrap();
1124+
parse_select(Dialect::PostgreSQL, "SELECT CAST(x AS public.abc) FROM t").unwrap();
11261125
assert!(registry
11271126
.add_query(RecipeExpr::Cache {
11281127
name: "foo".into(),
@@ -1150,25 +1149,25 @@ mod tests {
11501149
mod expr_skeleton {
11511150
use std::collections::HashMap;
11521151

1153-
use nom_sql::{parse_create_table, parse_select_statement};
1152+
use nom_sql::parse_create_table;
11541153
use readyset_sql::ast::{ItemPlaceholder, Literal};
11551154
use readyset_sql::Dialect;
1155+
use readyset_sql_parsing::parse_select;
11561156

11571157
use super::{recipe_expr_cache, ExprRegistry, ExprSkeletons, MatchedCache, RecipeExpr};
11581158

11591159
#[test]
11601160
fn equates_literals() {
11611161
let mut skeleton = ExprSkeletons::new();
11621162
skeleton.insert(
1163-
parse_select_statement(Dialect::MySQL, "SELECT NULL, true, 1 FROM t").unwrap(),
1163+
parse_select(Dialect::MySQL, "SELECT NULL, true, 1 FROM t").unwrap(),
11641164
"foo".into(),
11651165
);
11661166
// NULL is considered equivalent to itself in this context
11671167
assert_eq!(
11681168
skeleton
11691169
.caches_for_query(
1170-
parse_select_statement(Dialect::MySQL, "SELECT NULL, true, $1 FROM t")
1171-
.unwrap(),
1170+
parse_select(Dialect::MySQL, "SELECT NULL, true, $1 FROM t").unwrap(),
11721171
)
11731172
.unwrap(),
11741173
vec![MatchedCache {
@@ -1181,8 +1180,7 @@ mod tests {
11811180
assert_eq!(
11821181
skeleton
11831182
.caches_for_query(
1184-
parse_select_statement(Dialect::MySQL, "SELECT NULL, false, $1 FROM t")
1185-
.unwrap(),
1183+
parse_select(Dialect::MySQL, "SELECT NULL, false, $1 FROM t").unwrap(),
11861184
)
11871185
.unwrap(),
11881186
vec![]
@@ -1192,8 +1190,7 @@ mod tests {
11921190
assert_eq!(
11931191
skeleton
11941192
.caches_for_query(
1195-
parse_select_statement(Dialect::MySQL, "SELECT NULL, $1, 1.0 from t")
1196-
.unwrap()
1193+
parse_select(Dialect::MySQL, "SELECT NULL, $1, 1.0 from t").unwrap()
11971194
)
11981195
.unwrap(),
11991196
vec![]
@@ -1204,22 +1201,21 @@ mod tests {
12041201
fn finds_many() {
12051202
let mut skeleton = ExprSkeletons::new();
12061203
skeleton.insert(
1207-
parse_select_statement(Dialect::MySQL, "SELECT $1, NULL, 1 FROM t").unwrap(),
1204+
parse_select(Dialect::MySQL, "SELECT $1, NULL, 1 FROM t").unwrap(),
12081205
"foo".into(),
12091206
);
12101207
skeleton.insert(
1211-
parse_select_statement(Dialect::MySQL, "SELECT $1, NULL, 2 FROM t").unwrap(),
1208+
parse_select(Dialect::MySQL, "SELECT $1, NULL, 2 FROM t").unwrap(),
12121209
"bar".into(),
12131210
);
12141211
skeleton.insert(
1215-
parse_select_statement(Dialect::MySQL, "SELECT $1, $2, 2 FROM t").unwrap(),
1212+
parse_select(Dialect::MySQL, "SELECT $1, $2, 2 FROM t").unwrap(),
12161213
"baz".into(),
12171214
);
12181215

12191216
let res = skeleton
12201217
.caches_for_query(
1221-
parse_select_statement(Dialect::MySQL, "SELECT \"string\", NULL, $1 FROM t")
1222-
.unwrap(),
1218+
parse_select(Dialect::MySQL, "SELECT \"string\", NULL, $1 FROM t").unwrap(),
12231219
)
12241220
.unwrap();
12251221
let truth = vec![
@@ -1245,9 +1241,7 @@ mod tests {
12451241
assert_eq!(res, truth);
12461242

12471243
let res = skeleton
1248-
.caches_for_query(
1249-
parse_select_statement(Dialect::MySQL, "SELECT 1, 2, 2 FROM t").unwrap(),
1250-
)
1244+
.caches_for_query(parse_select(Dialect::MySQL, "SELECT 1, 2, 2 FROM t").unwrap())
12511245
.unwrap();
12521246
assert_eq!(
12531247
res,

‎readyset-server/src/integration.rs

+9-16
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ use dataflow::{
2626
};
2727
use futures::{join, StreamExt};
2828
use itertools::Itertools;
29-
use nom_sql::{parse_create_table, parse_create_view, parse_select_statement};
29+
use nom_sql::{parse_create_table, parse_create_view};
3030
use readyset_client::consensus::{Authority, LocalAuthority, LocalAuthorityStore};
3131
use readyset_client::recipe::changelist::{Change, ChangeList, CreateCache};
3232
use readyset_client::{KeyComparison, Modification, SchemaType, ViewPlaceholder, ViewQuery};
3333
use readyset_data::{Bound, DfType, DfValue, Dialect, IntoBoundedRange};
3434
use readyset_errors::ReadySetError::{self, RpcFailed, SelectQueryCreationFailed};
3535
use readyset_sql::ast;
3636
use readyset_sql::ast::{OrderType, Relation, SqlQuery};
37-
use readyset_sql_parsing::parse_query;
37+
use readyset_sql_parsing::{parse_query, parse_select};
3838
use readyset_util::eventually;
3939
use readyset_util::shutdown::ShutdownSender;
4040
use rust_decimal::prelude::ToPrimitive;
@@ -8773,11 +8773,8 @@ async fn multiple_simultaneous_migrations() {
87738773
Change::CreateCache(CreateCache {
87748774
name: Some("q1".into()),
87758775
statement: Box::new(
8776-
parse_select_statement(
8777-
readyset_sql::Dialect::MySQL,
8778-
"SELECT * FROM t WHERE x = ?"
8779-
)
8780-
.unwrap()
8776+
parse_select(readyset_sql::Dialect::MySQL, "SELECT * FROM t WHERE x = ?")
8777+
.unwrap()
87818778
),
87828779
always: false,
87838780
}),
@@ -8787,11 +8784,8 @@ async fn multiple_simultaneous_migrations() {
87878784
Change::CreateCache(CreateCache {
87888785
name: Some("q2".into()),
87898786
statement: Box::new(
8790-
parse_select_statement(
8791-
readyset_sql::Dialect::MySQL,
8792-
"SELECT * FROM t WHERE y = ?"
8793-
)
8794-
.unwrap()
8787+
parse_select(readyset_sql::Dialect::MySQL, "SELECT * FROM t WHERE y = ?")
8788+
.unwrap()
87958789
),
87968790
always: false,
87978791
}),
@@ -9231,7 +9225,7 @@ async fn views_out_of_order() {
92319225
g.extend_recipe(ChangeList::from_change(
92329226
Change::create_cache(
92339227
"q",
9234-
parse_select_statement(readyset_sql::Dialect::MySQL, "SELECT x FROM v2").unwrap(),
9228+
parse_select(readyset_sql::Dialect::MySQL, "SELECT x FROM v2").unwrap(),
92359229
false,
92369230
),
92379231
Dialect::DEFAULT_MYSQL,
@@ -9260,8 +9254,7 @@ async fn evict_single() {
92609254
g.extend_recipe(ChangeList::from_change(
92619255
Change::create_cache(
92629256
"q",
9263-
parse_select_statement(readyset_sql::Dialect::MySQL, "SELECT x FROM t1 where y = ?")
9264-
.unwrap(),
9257+
parse_select(readyset_sql::Dialect::MySQL, "SELECT x FROM t1 where y = ?").unwrap(),
92659258
false,
92669259
),
92679260
Dialect::DEFAULT_MYSQL,
@@ -9324,7 +9317,7 @@ async fn evict_single_intermediate_state() {
93249317
g.extend_recipe(ChangeList::from_change(
93259318
Change::create_cache(
93269319
"q",
9327-
parse_select_statement(
9320+
parse_select(
93289321
readyset_sql::Dialect::MySQL,
93299322
"SELECT sum(x) FROM t1 WHERE y = ?",
93309323
)

‎readyset-sql-parsing/src/lib.rs

+29-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
use readyset_sql::{
2-
ast::{Expr, SqlQuery},
3-
Dialect,
4-
};
1+
use readyset_sql::ast::{Expr, SelectStatement, SqlQuery};
2+
use readyset_sql::Dialect;
53

64
#[cfg(feature = "sqlparser")]
75
use readyset_sql::{
@@ -564,7 +562,34 @@ pub fn parse_expr(dialect: Dialect, input: impl AsRef<str>) -> Result<Expr, Stri
564562
)
565563
}
566564

565+
#[cfg(feature = "sqlparser")]
566+
fn parse_readyset_select(
567+
parser: &mut Parser,
568+
dialect: Dialect,
569+
_input: impl AsRef<str>,
570+
) -> Result<SelectStatement, ReadysetParsingError> {
571+
// SQLParser has this weird behaviour where `parse_select` subparser
572+
// doesn't parse limits or offsets....
573+
Ok(parser.parse_query()?.try_into_dialect(dialect)?)
574+
}
575+
567576
#[cfg(not(feature = "sqlparser"))]
568577
pub fn parse_expr(dialect: Dialect, input: impl AsRef<str>) -> Result<Expr, String> {
569578
nom_sql::parse_expr(dialect, input.as_ref())
570579
}
580+
581+
#[cfg(not(feature = "sqlparser"))]
582+
pub fn parse_select(dialect: Dialect, input: impl AsRef<str>) -> Result<SelectStatement, String> {
583+
nom_sql::parse_select_statement(dialect, input.as_ref())
584+
}
585+
586+
/// Parses a single expression; only intended for use in tests.
587+
#[cfg(feature = "sqlparser")]
588+
pub fn parse_select(dialect: Dialect, input: impl AsRef<str>) -> Result<SelectStatement, String> {
589+
parse_both_inner(
590+
dialect,
591+
input,
592+
|d, s| nom_sql::parse_select_statement(d, s),
593+
|p, d, s| parse_readyset_select(p, d, s),
594+
)
595+
}

‎readyset-sql-passes/src/anonymize.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ mod tests {
221221
use super::*;
222222

223223
fn parse_select_statement(q: &str) -> SelectStatement {
224-
nom_sql::parse_select_statement(Dialect::MySQL, q).unwrap()
224+
readyset_sql_parsing::parse_select(Dialect::MySQL, q).unwrap()
225225
}
226226

227227
fn parse_create_table_statement(q: &str) -> CreateTableStatement {

0 commit comments

Comments
 (0)