Skip to content

Commit

Permalink
fix: fucntion substring for out of bounds error (KipData#161)
Browse files Browse the repository at this point in the history
* fix: [substring bug](KipData#160)

* fix: `substring.slt` case
  • Loading branch information
KKould authored Mar 14, 2024
1 parent be5cad5 commit 937ec7e
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 26 deletions.
4 changes: 3 additions & 1 deletion src/binder/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ impl<'a, T: Transaction> Binder<'a, T> {
}
Ok(UnionOperator::build(
left_schema.clone(),
right_schema.clone(),
left_plan,
right_plan,
))
Expand All @@ -192,7 +193,8 @@ impl<'a, T: Transaction> Binder<'a, T> {
));
}
let union_op = Operator::Union(UnionOperator {
schema_ref: left_schema.clone(),
left_schema_ref: left_schema.clone(),
_right_schema_ref: right_schema.clone(),
});
let distinct_exprs = left_schema
.iter()
Expand Down
19 changes: 15 additions & 4 deletions src/expression/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::types::value::{DataValue, ValueRef};
use crate::types::LogicalType;
use itertools::Itertools;
use lazy_static::lazy_static;
use std::cmp;
use std::cmp::Ordering;
use std::sync::Arc;

Expand All @@ -20,7 +21,7 @@ macro_rules! eval_to_num {
.cast(&LogicalType::Integer)?
.i32()
{
num_i32 as usize
num_i32
} else {
return Ok(Arc::new(DataValue::Utf8(None)));
}
Expand Down Expand Up @@ -156,11 +157,21 @@ impl ScalarExpression {
.utf8()
{
if let Some(from_expr) = from_expr {
string = string
.split_off(eval_to_num!(from_expr, tuple, schema).saturating_sub(1));
let mut from = eval_to_num!(from_expr, tuple, schema).saturating_sub(1);
let len_i = string.len() as i32;

while from < 0 {
from += len_i + 1;
}
if from > len_i {
return Ok(Arc::new(DataValue::Utf8(None)));
}
string = string.split_off(from as usize);
}
if let Some(for_expr) = for_expr {
let _ = string.split_off(eval_to_num!(for_expr, tuple, schema));
let for_i =
cmp::min(eval_to_num!(for_expr, tuple, schema) as usize, string.len());
let _ = string.split_off(for_i);
}

Ok(Arc::new(DataValue::Utf8(Some(string))))
Expand Down
5 changes: 4 additions & 1 deletion src/planner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ impl LogicalPlan {
Arc::new(out_columns)
}
Operator::Values(ValuesOperator { schema_ref, .. })
| Operator::Union(UnionOperator { schema_ref }) => schema_ref.clone(),
| Operator::Union(UnionOperator {
left_schema_ref: schema_ref,
..
}) => schema_ref.clone(),
Operator::Dummy => Arc::new(vec![]),
Operator::Show => Arc::new(vec![
Arc::new(ColumnCatalog::new_dummy("TABLE".to_string())),
Expand Down
16 changes: 13 additions & 3 deletions src/planner/operator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ impl Operator {
),
Operator::Sort(_) | Operator::Limit(_) => None,
Operator::Values(ValuesOperator { schema_ref, .. })
| Operator::Union(UnionOperator { schema_ref }) => Some(
| Operator::Union(UnionOperator {
left_schema_ref: schema_ref,
..
}) => Some(
schema_ref
.iter()
.cloned()
Expand Down Expand Up @@ -197,8 +200,15 @@ impl Operator {
.map(|field| &field.expr)
.flat_map(|expr| expr.referenced_columns(only_column_ref))
.collect_vec(),
Operator::Values(ValuesOperator { schema_ref, .. })
| Operator::Union(UnionOperator { schema_ref }) => Vec::clone(schema_ref),
Operator::Values(ValuesOperator { schema_ref, .. }) => Vec::clone(schema_ref),
Operator::Union(UnionOperator {
left_schema_ref,
_right_schema_ref,
}) => left_schema_ref
.iter()
.chain(_right_schema_ref.iter())
.cloned()
.collect_vec(),
Operator::Analyze(_) => vec![],
Operator::Delete(op) => vec![op.primary_key_column.clone()],
Operator::Dummy
Expand Down
14 changes: 10 additions & 4 deletions src/planner/operator/union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,23 @@ use std::fmt::Formatter;

#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub struct UnionOperator {
pub schema_ref: SchemaRef,
pub left_schema_ref: SchemaRef,
// mainly use `left_schema` as output and `right_schema` for `column pruning`
pub _right_schema_ref: SchemaRef,
}

impl UnionOperator {
pub fn build(
schema_ref: SchemaRef,
left_schema_ref: SchemaRef,
right_schema_ref: SchemaRef,
left_plan: LogicalPlan,
right_plan: LogicalPlan,
) -> LogicalPlan {
LogicalPlan::new(
Operator::Union(UnionOperator { schema_ref }),
Operator::Union(UnionOperator {
left_schema_ref,
_right_schema_ref: right_schema_ref,
}),
vec![left_plan, right_plan],
)
}
Expand All @@ -26,7 +32,7 @@ impl UnionOperator {
impl fmt::Display for UnionOperator {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
let schema = self
.schema_ref
.left_schema_ref
.iter()
.map(|column| column.name().to_string())
.join(", ");
Expand Down
23 changes: 21 additions & 2 deletions tests/slt/substring → tests/slt/substring.slt
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
query T
select substring('pineapple' from 5 for 10 )
----
app
apple

query T
select substring('pineapple' from -5 for 10 )
----
apple

query T
select substring('pineapple', -15, 10 )
----
apple

query T
select substring('pineapple' for 4 )
Expand All @@ -16,11 +26,20 @@ apple
query T
select substring('pineapple' from 1 for null )
----
null

query T
select substring('pineapple' from null for 4 )
----
null

query T
select substring(null from 1 for 4 )
----
----
null

# issue: https://github.com/KipData/FnckSQL/issues/160
query T
select substring('abc', 1, 10);
----
abc
10 changes: 5 additions & 5 deletions tests/slt/tuple → tests/slt/tuple.slt
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,16 @@ select (id, v1) from t1;
(4, 4)

query T
select * from t1 where (id,v1) == (1,1);
select * from t1 where (id,v1) = (1,1);
----
1 1

query T
select * from t1 where (id,v1) != (1,1);
----
(2, 2)
(3, 3)
(4, 4)
2 2
3 3
4 4

query T
select * from t1 where (id,v1) in ((1,1), (2, 2));
Expand All @@ -57,4 +57,4 @@ select * from t1 where (id,v1) not in ((1,1), (2, 2));
4 4

statement ok
drop t1
drop table t1
12 changes: 7 additions & 5 deletions tests/slt/union → tests/slt/union.slt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ select 1 union all select 1
1
1

query T
query T rowsort
select (1, 2) union select (2, 1) union select (1, 2)
----
(1, 2)
Expand All @@ -33,15 +33,17 @@ create table t1(id int primary key, v1 int unique)
statement ok
insert into t1 values (1,1), (2,2), (3,3), (4,4)

query I
select v1 from t1 union select * from t1
query I rowsort
select id from t1 union select v1 from t1
----
1
2
3
4

query I rowsort
select v1 from t1 union all select * from t1
select id from t1 union all select v1 from t1
----
1
1
2
Expand All @@ -52,4 +54,4 @@ select v1 from t1 union all select * from t1
4

statement ok
drop t1
drop table t1
5 changes: 4 additions & 1 deletion tests/slt/update → tests/slt/update.slt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ update t set v2 = 9 where v1 = 1
query IIII rowsort
select * from t;
----
0 1 9 100
1 1 9 100
2 2 20 200
3 3 30 300
Expand All @@ -24,17 +25,19 @@ update t set v2 = 9
query IIII rowsort
select * from t
----
0 1 9 100
1 1 9 100
2 2 9 200
3 3 9 300
4 4 9 400

statement ok
update t set v2 = default
update t set v3 = default

query IIII rowsort
select * from t
----
0 1 9 233
1 1 9 233
2 2 9 233
3 3 9 233
Expand Down

0 comments on commit 937ec7e

Please sign in to comment.