Skip to content

Commit

Permalink
fix: CONCAT() to support multiple arguments (#690)
Browse files Browse the repository at this point in the history 
* fix: concat multiple strings

* clippy fix
  • Loading branch information
@snork-alt
snork-alt committed Jan 20, 2023
1 parent 69bfd42 commit 4047843
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 60 deletions.
13 changes: 2 additions & 11 deletions dozer-sql/src/pipeline/expression/scalar/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,7 @@ pub(crate) fn get_scalar_function_type(
ScalarFunctionType::Ucase => {
validate_ucase(argv!(args, 0, ScalarFunctionType::Ucase)?, schema)
}
ScalarFunctionType::Concat => validate_concat(
argv!(args, 0, ScalarFunctionType::Concat)?,
argv!(args, 1, ScalarFunctionType::Concat)?,
schema,
),
ScalarFunctionType::Concat => validate_concat(args, schema),
ScalarFunctionType::Length => Ok(ExpressionType::new(FieldType::UInt, false)),
}
}
Expand Down Expand Up @@ -82,12 +78,7 @@ impl ScalarFunctionType {
ScalarFunctionType::Ucase => {
evaluate_ucase(schema, argv!(args, 0, ScalarFunctionType::Ucase)?, record)
}
ScalarFunctionType::Concat => evaluate_concat(
schema,
argv!(args, 0, ScalarFunctionType::Concat)?,
argv!(args, 1, ScalarFunctionType::Concat)?,
record,
),
ScalarFunctionType::Concat => evaluate_concat(schema, args, record),
ScalarFunctionType::Length => {
evaluate_length(schema, argv!(args, 0, ScalarFunctionType::Length)?, record)
}
Expand Down
79 changes: 34 additions & 45 deletions dozer-sql/src/pipeline/expression/scalar/string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::pipeline::expression::execution::{Expression, ExpressionExecutor, Exp

use crate::pipeline::expression::arg_utils::validate_arg_type;
use crate::pipeline::expression::scalar::common::ScalarFunctionType;

use dozer_types::types::{Field, FieldType, Record, Schema};
use like::{Escape, Like};

Expand Down Expand Up @@ -38,59 +39,47 @@ pub(crate) fn evaluate_ucase(
}

pub(crate) fn validate_concat(
arg0: &Expression,
arg1: &Expression,
args: &[Expression],
schema: &Schema,
) -> Result<ExpressionType, PipelineError> {
let arg0 = validate_arg_type(
arg0,
vec![FieldType::String, FieldType::Text],
schema,
ScalarFunctionType::Concat,
0,
)?;
let arg1 = validate_arg_type(
arg1,
vec![FieldType::String, FieldType::Text],
schema,
ScalarFunctionType::Concat,
1,
)?;

Ok(ExpressionType::new(
match (arg0.return_type, arg1.return_type) {
(FieldType::String, FieldType::String) => FieldType::String,
_ => FieldType::Text,
},
false,
))
let mut ret_type = FieldType::String;
for exp in args {
let r = validate_arg_type(
exp,
vec![FieldType::String, FieldType::Text],
schema,
ScalarFunctionType::Concat,
0,
)?;
if matches!(r.return_type, FieldType::Text) {
ret_type = FieldType::Text;
}
}
Ok(ExpressionType::new(ret_type, false))
}

pub(crate) fn evaluate_concat(
schema: &Schema,
arg0: &Expression,
arg1: &Expression,
args: &[Expression],
record: &Record,
) -> Result<Field, PipelineError> {
let (f0, f1) = (
arg0.evaluate(record, schema)?,
arg1.evaluate(record, schema)?,
);
let (v0, v1) = (
arg_str!(f0, ScalarFunctionType::Concat, 0)?,
arg_str!(f1, ScalarFunctionType::Concat, 1)?,
);
let ret_val = v0 + v1.as_str();

Ok(
match (
arg0.get_type(schema)?.return_type,
arg1.get_type(schema)?.return_type,
) {
(FieldType::String, FieldType::String) => Field::String(ret_val),
_ => Field::Text(ret_val),
},
)
let mut res_type = FieldType::String;
let mut res_vec: Vec<String> = Vec::with_capacity(args.len());

for e in args {
if matches!(e.get_type(schema)?.return_type, FieldType::Text) {
res_type = FieldType::Text;
}
let f = e.evaluate(record, schema)?;
let val = arg_str!(f, ScalarFunctionType::Concat, 0)?;
res_vec.push(val);
}

let res_str = res_vec.iter().fold(String::new(), |a, b| a + b.as_str());
Ok(match res_type {
FieldType::Text => Field::Text(res_str),
_ => Field::String(res_str),
})
}

pub(crate) fn evaluate_length(
Expand Down
30 changes: 26 additions & 4 deletions dozer-sql/src/pipeline/expression/scalar/tests/string.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use dozer_types::types::{Field, FieldDefinition, FieldType, Record, Schema};
#[test]
fn test_concat() {
let f = run_scalar_fct(
"SELECT CONCAT(fn, ln) FROM USERS",
"SELECT CONCAT(fn, ln, fn) FROM USERS",
Schema::empty()
.field(
FieldDefinition::new(String::from("fn"), FieldType::String, false),
Expand All @@ -23,13 +23,13 @@ fn test_concat() {
Field::String("Doe".to_string()),
],
);
assert_eq!(f, Field::String("JohnDoe".to_string()));
assert_eq!(f, Field::String("JohnDoeJohn".to_string()));
}

#[test]
fn test_concat_text() {
let f = run_scalar_fct(
"SELECT CONCAT(fn, ln) FROM USERS",
"SELECT CONCAT(fn, ln, fn) FROM USERS",
Schema::empty()
.field(
FieldDefinition::new(String::from("fn"), FieldType::Text, false),
Expand All @@ -45,7 +45,29 @@ fn test_concat_text() {
Field::String("Doe".to_string()),
],
);
assert_eq!(f, Field::Text("JohnDoe".to_string()));
assert_eq!(f, Field::Text("JohnDoeJohn".to_string()));
}

#[test]
fn test_concat_text_empty() {
let f = run_scalar_fct(
"SELECT CONCAT() FROM USERS",
Schema::empty()
.field(
FieldDefinition::new(String::from("fn"), FieldType::String, false),
false,
)
.field(
FieldDefinition::new(String::from("ln"), FieldType::String, false),
false,
)
.clone(),
vec![
Field::String("John".to_string()),
Field::String("Doe".to_string()),
],
);
assert_eq!(f, Field::String("".to_string()));
}

#[test]
Expand Down

0 comments on commit 4047843

Please sign in to comment.