Skip to content

Commit 036385d

Browse files
KKouldKKould
authored
Fixed and added multiple e2e tests (#70)
* fix: sqllogictest * test: add e2e test of `join.slt` * test: add e2e test of `aggregation.slt` * test: add e2e test of `basic_test.slt` and `count.slt` * test: add e2e test of `create.slt`, `delete.slt`, `distinct.slt`, `filter.slt`, `filter_null.slt` * test: add e2e test of `group_by.slt` * test: add e2e test of `having.slt` * test: add e2e test of `order_by.slt` * fix: test compilation
1 parent 6691037 commit 036385d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1065
-374
lines changed

‎src/binder/aggregate.rs‎

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ impl<S: Storage> Binder<S> {
3030
select_items: &mut [ScalarExpression],
3131
) -> Result<(), BindError> {
3232
for column in select_items {
33-
self.visit_column_agg_expr(column);
33+
self.visit_column_agg_expr(column, true)?;
3434
}
3535
Ok(())
3636
}
@@ -57,7 +57,8 @@ impl<S: Storage> Binder<S> {
5757
// Extract having expression.
5858
let return_having = if let Some(having) = having {
5959
let mut having = self.bind_expr(having).await?;
60-
self.visit_column_agg_expr(&mut having);
60+
self.visit_column_agg_expr(&mut having, false)?;
61+
6162
Some(having)
6263
} else {
6364
None
@@ -73,11 +74,11 @@ impl<S: Storage> Binder<S> {
7374
nulls_first,
7475
} = orderby;
7576
let mut expr = self.bind_expr(expr).await?;
76-
self.visit_column_agg_expr(&mut expr);
77+
self.visit_column_agg_expr(&mut expr, false)?;
7778

7879
return_orderby.push(SortField::new(
7980
expr,
80-
asc.map_or(true, |asc| !asc),
81+
asc.map_or(true, |asc| asc),
8182
nulls_first.map_or(false, |first| first),
8283
));
8384
}
@@ -88,50 +89,67 @@ impl<S: Storage> Binder<S> {
8889
Ok((return_having, return_orderby))
8990
}
9091

91-
fn visit_column_agg_expr(&mut self, expr: &mut ScalarExpression) {
92+
fn visit_column_agg_expr(&mut self, expr: &mut ScalarExpression, is_select: bool) -> Result<(), BindError> {
9293
match expr {
9394
ScalarExpression::AggCall {
9495
ty: return_type, ..
9596
} => {
96-
let index = self.context.input_ref_index(InputRefType::AggCall);
97-
let input_ref = ScalarExpression::InputRef {
98-
index,
99-
ty: return_type.clone(),
100-
};
101-
match std::mem::replace(expr, input_ref) {
102-
ScalarExpression::AggCall {
103-
kind,
104-
args,
97+
let ty = return_type.clone();
98+
if is_select {
99+
let index = self.context.input_ref_index(InputRefType::AggCall);
100+
let input_ref = ScalarExpression::InputRef {
101+
index,
105102
ty,
106-
distinct
107-
} => {
108-
self.context.agg_calls.push(ScalarExpression::AggCall {
109-
distinct,
103+
};
104+
match std::mem::replace(expr, input_ref) {
105+
ScalarExpression::AggCall {
110106
kind,
111107
args,
112108
ty,
113-
});
109+
distinct
110+
} => {
111+
self.context.agg_calls.push(ScalarExpression::AggCall {
112+
distinct,
113+
kind,
114+
args,
115+
ty,
116+
});
117+
}
118+
_ => unreachable!(),
114119
}
115-
_ => unreachable!(),
120+
} else {
121+
let (index, _) = self
122+
.context
123+
.agg_calls
124+
.iter()
125+
.find_position(|agg_expr| agg_expr == &expr)
126+
.ok_or_else(|| BindError::AggMiss(format!("{:?}", expr)))?;
127+
128+
let _ = std::mem::replace(expr, ScalarExpression::InputRef {
129+
index,
130+
ty,
131+
});
116132
}
117133
}
118134

119-
ScalarExpression::TypeCast { expr, .. } => self.visit_column_agg_expr(expr),
120-
ScalarExpression::IsNull { expr } => self.visit_column_agg_expr(expr),
121-
ScalarExpression::Unary { expr, .. } => self.visit_column_agg_expr(expr),
122-
ScalarExpression::Alias { expr, .. } => self.visit_column_agg_expr(expr),
135+
ScalarExpression::TypeCast { expr, .. } => self.visit_column_agg_expr(expr, is_select)?,
136+
ScalarExpression::IsNull { expr } => self.visit_column_agg_expr(expr, is_select)?,
137+
ScalarExpression::Unary { expr, .. } => self.visit_column_agg_expr(expr, is_select)?,
138+
ScalarExpression::Alias { expr, .. } => self.visit_column_agg_expr(expr, is_select)?,
123139
ScalarExpression::Binary {
124140
left_expr,
125141
right_expr,
126142
..
127143
} => {
128-
self.visit_column_agg_expr(left_expr);
129-
self.visit_column_agg_expr(right_expr);
144+
self.visit_column_agg_expr(left_expr, is_select)?;
145+
self.visit_column_agg_expr(right_expr, is_select)?;
130146
}
131147
ScalarExpression::Constant(_)
132148
| ScalarExpression::ColumnRef { .. }
133149
| ScalarExpression::InputRef { .. } => {}
134150
}
151+
152+
Ok(())
135153
}
136154

137155
/// Validate select exprs must appear in the GROUP BY clause or be used in
@@ -173,6 +191,7 @@ impl<S: Storage> Binder<S> {
173191
if expr.has_agg_call(&self.context) {
174192
continue;
175193
}
194+
176195
group_raw_set.remove(expr);
177196

178197
if !group_raw_exprs.iter().contains(expr) {
@@ -271,6 +290,9 @@ impl<S: Storage> Binder<S> {
271290
if self.context.group_by_exprs.contains(expr) {
272291
return Ok(());
273292
}
293+
if matches!(expr, ScalarExpression::Alias { .. }) {
294+
return self.validate_having_orderby(expr.unpack_alias());
295+
}
274296

275297
Err(BindError::AggMiss(
276298
format!(

‎src/binder/create_table.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::HashSet;
22
use std::sync::Arc;
33
use itertools::Itertools;
4-
use sqlparser::ast::{ColumnDef, ObjectName};
4+
use sqlparser::ast::{ColumnDef, ObjectName, TableConstraint};
55

66
use super::Binder;
77
use crate::binder::{BindError, lower_case_name, split_name};
@@ -12,10 +12,12 @@ use crate::planner::operator::Operator;
1212
use crate::storage::Storage;
1313

1414
impl<S: Storage> Binder<S> {
15+
// TODO: TableConstraint
1516
pub(crate) fn bind_create_table(
1617
&mut self,
1718
name: &ObjectName,
1819
columns: &[ColumnDef],
20+
constraints: &[TableConstraint]
1921
) -> Result<LogicalPlan, BindError> {
2022
let name = lower_case_name(&name);
2123
let (_, name) = split_name(&name)?;

‎src/binder/expr.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ impl<S: Storage> Binder<S> {
8585
}
8686
if got_column.is_none() {
8787
if let Some(expr) = self.context.aliases.get(column_name) {
88-
return Ok(expr.clone());
88+
return Ok(ScalarExpression::Alias { expr: Box::new(expr.clone()), alias: column_name.clone() });
8989
}
9090
}
9191
let column_catalog =
@@ -167,7 +167,7 @@ impl<S: Storage> Binder<S> {
167167
distinct: func.distinct,
168168
kind: AggKind::Count,
169169
args,
170-
ty: LogicalType::UInteger,
170+
ty: LogicalType::Integer,
171171
},
172172
"sum" => ScalarExpression::AggCall{
173173
distinct: func.distinct,

‎src/binder/mod.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ impl<S: Storage> Binder<S> {
8282
pub async fn bind(mut self, stmt: &Statement) -> Result<LogicalPlan, BindError> {
8383
let plan = match stmt {
8484
Statement::Query(query) => self.bind_query(query).await?,
85-
Statement::CreateTable { name, columns, .. } => self.bind_create_table(name, &columns)?,
85+
Statement::CreateTable { name, columns, constraints, .. } => {
86+
self.bind_create_table(name, &columns, &constraints)?
87+
},
8688
Statement::Drop { object_type, names, .. } => {
8789
match object_type {
8890
ObjectType::Table => {
@@ -168,9 +170,9 @@ pub enum BindError {
168170
SubqueryMustHaveAlias,
169171
#[error("agg miss: {0}")]
170172
AggMiss(String),
171-
#[error("catalog error")]
173+
#[error("catalog error: {0}")]
172174
CatalogError(#[from] CatalogError),
173-
#[error("type error")]
175+
#[error("type error: {0}")]
174176
TypeError(#[from] TypeError)
175177
}
176178

@@ -193,16 +195,16 @@ pub mod test {
193195
let _ = storage.create_table(
194196
Arc::new("t1".to_string()),
195197
vec![
196-
ColumnCatalog::new("c1".to_string(), false, ColumnDesc::new(Integer, true, false)),
197-
ColumnCatalog::new("c2".to_string(), false, ColumnDesc::new(Integer, false, true)),
198+
ColumnCatalog::new("c1".to_string(), false, ColumnDesc::new(Integer, true, false), None),
199+
ColumnCatalog::new("c2".to_string(), false, ColumnDesc::new(Integer, false, true), None),
198200
]
199201
).await?;
200202

201203
let _ = storage.create_table(
202204
Arc::new("t2".to_string()),
203205
vec![
204-
ColumnCatalog::new("c3".to_string(), false, ColumnDesc::new(Integer, true, false)),
205-
ColumnCatalog::new("c4".to_string(), false, ColumnDesc::new(Integer, false, false)),
206+
ColumnCatalog::new("c3".to_string(), false, ColumnDesc::new(Integer, true, false), None),
207+
ColumnCatalog::new("c4".to_string(), false, ColumnDesc::new(Integer, false, false), None),
206208
]
207209
).await?;
208210

‎src/catalog/column.rs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::sync::Arc;
22
use serde::{Deserialize, Serialize};
33
use sqlparser::ast::{ColumnDef, ColumnOption};
44
use crate::catalog::TableName;
5+
use crate::expression::ScalarExpression;
56

67
use crate::types::{ColumnId, LogicalType};
78

@@ -14,16 +15,23 @@ pub struct ColumnCatalog {
1415
pub table_name: Option<TableName>,
1516
pub nullable: bool,
1617
pub desc: ColumnDesc,
18+
pub ref_expr: Option<ScalarExpression>,
1719
}
1820

1921
impl ColumnCatalog {
20-
pub(crate) fn new(column_name: String, nullable: bool, column_desc: ColumnDesc) -> ColumnCatalog {
22+
pub(crate) fn new(
23+
column_name: String,
24+
nullable: bool,
25+
column_desc: ColumnDesc,
26+
ref_expr: Option<ScalarExpression>
27+
) -> ColumnCatalog {
2128
ColumnCatalog {
2229
id: None,
2330
name: column_name,
2431
table_name: None,
2532
nullable,
2633
desc: column_desc,
34+
ref_expr,
2735
}
2836
}
2937

@@ -34,6 +42,7 @@ impl ColumnCatalog {
3442
table_name: None,
3543
nullable: false,
3644
desc: ColumnDesc::new(LogicalType::Varchar(None), false, false),
45+
ref_expr: None,
3746
}
3847
}
3948

@@ -75,7 +84,7 @@ impl From<ColumnDef> for ColumnCatalog {
7584
}
7685
}
7786

78-
ColumnCatalog::new(column_name, nullable, column_desc)
87+
ColumnCatalog::new(column_name, nullable, column_desc, None)
7988
}
8089
}
8190

‎src/catalog/root.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,13 @@ mod tests {
6868
"a".to_string(),
6969
false,
7070
ColumnDesc::new(LogicalType::Integer, false, false),
71+
None
7172
);
7273
let col1 = ColumnCatalog::new(
7374
"b".to_string(),
7475
false,
7576
ColumnDesc::new(LogicalType::Boolean, false, false),
77+
None
7678
);
7779
let col_catalogs = vec![col0, col1];
7880

‎src/catalog/table.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ impl TableCatalog {
2323
.find(|meta| meta.is_unique && &meta.column_ids[0] == col_id)
2424
}
2525

26+
#[allow(dead_code)]
2627
pub(crate) fn get_column_by_id(&self, id: &ColumnId) -> Option<&ColumnRef> {
2728
self.columns.get(id)
2829
}
2930

31+
#[allow(dead_code)]
3032
pub(crate) fn get_column_id_by_name(&self, name: &String) -> Option<ColumnId> {
3133
self.column_idxs.get(name).cloned()
3234
}
@@ -123,8 +125,8 @@ mod tests {
123125
// | 1 | true |
124126
// | 2 | false |
125127
fn test_table_catalog() {
126-
let col0 = ColumnCatalog::new("a".into(), false, ColumnDesc::new(LogicalType::Integer, false, false));
127-
let col1 = ColumnCatalog::new("b".into(), false, ColumnDesc::new(LogicalType::Boolean, false, false));
128+
let col0 = ColumnCatalog::new("a".into(), false, ColumnDesc::new(LogicalType::Integer, false, false), None);
129+
let col1 = ColumnCatalog::new("b".into(), false, ColumnDesc::new(LogicalType::Boolean, false, false), None);
128130
let col_catalogs = vec![col0, col1];
129131
let table_catalog = TableCatalog::new(Arc::new("test".to_string()), col_catalogs).unwrap();
130132

‎src/db.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,12 +168,14 @@ mod test {
168168
ColumnCatalog::new(
169169
"c1".to_string(),
170170
false,
171-
ColumnDesc::new(LogicalType::Integer, true, false)
171+
ColumnDesc::new(LogicalType::Integer, true, false),
172+
None
172173
),
173174
ColumnCatalog::new(
174175
"c2".to_string(),
175176
false,
176-
ColumnDesc::new(LogicalType::Boolean, false, false)
177+
ColumnDesc::new(LogicalType::Boolean, false, false),
178+
None
177179
),
178180
];
179181

‎src/execution/executor/dql/aggregate/count.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::execution::ExecutorError;
66
use crate::types::value::{DataValue, ValueRef};
77

88
pub struct CountAccumulator {
9-
result: u32,
9+
result: i32,
1010
}
1111

1212
impl CountAccumulator {
@@ -25,7 +25,7 @@ impl Accumulator for CountAccumulator {
2525
}
2626

2727
fn evaluate(&self) -> Result<ValueRef, ExecutorError> {
28-
Ok(Arc::new(DataValue::UInt32(Some(self.result))))
28+
Ok(Arc::new(DataValue::Int32(Some(self.result))))
2929
}
3030
}
3131

@@ -51,6 +51,6 @@ impl Accumulator for DistinctCountAccumulator {
5151
}
5252

5353
fn evaluate(&self) -> Result<ValueRef, ExecutorError> {
54-
Ok(Arc::new(DataValue::UInt32(Some(self.distinct_values.len() as u32))))
54+
Ok(Arc::new(DataValue::Int32(Some(self.distinct_values.len() as i32))))
5555
}
5656
}

‎src/execution/executor/dql/aggregate/hash_agg.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,9 @@ mod test {
125125
let desc = ColumnDesc::new(LogicalType::Integer, false, false);
126126

127127
let t1_columns = vec![
128-
Arc::new(ColumnCatalog::new("c1".to_string(), true, desc.clone())),
129-
Arc::new(ColumnCatalog::new("c2".to_string(), true, desc.clone())),
130-
Arc::new(ColumnCatalog::new("c3".to_string(), true, desc.clone())),
128+
Arc::new(ColumnCatalog::new("c1".to_string(), true, desc.clone(), None)),
129+
Arc::new(ColumnCatalog::new("c2".to_string(), true, desc.clone(), None)),
130+
Arc::new(ColumnCatalog::new("c3".to_string(), true, desc.clone(), None)),
131131
];
132132

133133
let operator = AggregateOperator {

0 commit comments

Comments
 (0)