Skip to content

Commit

Permalink
fix(query): fix udf name with upper case letters (databendlabs#15300)
Browse files Browse the repository at this point in the history
  • Loading branch information
@b41sh
b41sh authored Apr 23, 2024
1 parent f8b6190 commit e5e2d5c
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 24 deletions.
25 changes: 9 additions & 16 deletions src/query/functions/src/scalars/geometry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ pub fn register(registry: &mut FunctionRegistry) {
builder.len(),
ErrorCode::GeometryError(e.to_string()).to_string(),
);
builder.commit_row();
return;
}
};
builder.commit_row();
Expand Down Expand Up @@ -119,7 +117,6 @@ pub fn register(registry: &mut FunctionRegistry) {
builder.len(),
ErrorCode::GeometryError(e.to_string()).to_string(),
);
builder.put_str("");
builder.commit_row();
return;
}
Expand Down Expand Up @@ -147,8 +144,13 @@ pub fn register(registry: &mut FunctionRegistry) {
}
}
if geohash.len() > 12 {
ctx.set_error(builder.len(), "");
builder.put_str("Currently the precision only implement within 12 digits!");
ctx.set_error(
builder.len(),
ErrorCode::GeometryError(
"Currently the precision only implement within 12 digits!".to_string(),
)
.to_string(),
);
builder.commit_row();
return;
}
Expand All @@ -160,7 +162,6 @@ pub fn register(registry: &mut FunctionRegistry) {
builder.len(),
ErrorCode::GeometryError(e.to_string()).to_string(),
);
builder.put_str("");
builder.commit_row();
return;
}
Expand Down Expand Up @@ -218,7 +219,6 @@ pub fn register(registry: &mut FunctionRegistry) {
builder.len(),
ErrorCode::GeometryError(e.to_string()).to_string(),
);
builder.put_str("");
builder.commit_row();
return;
}
Expand Down Expand Up @@ -278,15 +278,13 @@ pub fn register(registry: &mut FunctionRegistry) {
},
Err(e) => {
ctx.set_error(builder.len(), ErrorCode::GeometryError(e).to_string());
builder.put_str("");
builder.commit_row();
return;
}
}
},
Err(e) => {
ctx.set_error(builder.len(), ErrorCode::GeometryError(e.to_string()).to_string());
builder.put_str("");
builder.commit_row();
return;
}
Expand All @@ -301,7 +299,6 @@ pub fn register(registry: &mut FunctionRegistry) {
Ok(point) => point,
Err(e) => {
ctx.set_error(builder.len(), ErrorCode::GeometryError(e.to_string()).to_string());
builder.put_str("");
builder.commit_row();
return;
}
Expand All @@ -312,8 +309,7 @@ pub fn register(registry: &mut FunctionRegistry) {
let line: LineString = match g.try_into() {
Ok(line) => line,
Err(e) => {
ctx.set_error(builder.len(), e.to_string());
builder.put_str("");
ctx.set_error(builder.len(), ErrorCode::GeometryError(e.to_string()).to_string());
builder.commit_row();
return;
}
Expand All @@ -324,8 +320,7 @@ pub fn register(registry: &mut FunctionRegistry) {
let multi_point: MultiPoint = match g.try_into() {
Ok(multi_point) => multi_point,
Err(e) => {
ctx.set_error(builder.len(), e.to_string());
builder.put_str("");
ctx.set_error(builder.len(), ErrorCode::GeometryError(e.to_string()).to_string());
builder.commit_row();
return;
}
Expand All @@ -339,7 +334,6 @@ pub fn register(registry: &mut FunctionRegistry) {
builder.len(),
ErrorCode::GeometryError("Geometry expression must be a Point, MultiPoint, or LineString.").to_string(),
);
builder.put_str("");
builder.commit_row();
return;
}
Expand Down Expand Up @@ -570,7 +564,6 @@ pub fn register(registry: &mut FunctionRegistry) {
builder.len(),
ErrorCode::GeometryError(e.to_string()).to_string(),
);
builder.put_str("");
}
}
builder.commit_row();
Expand Down
6 changes: 1 addition & 5 deletions src/query/sql/src/planner/binder/binder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ use crate::plans::DropConnectionPlan;
use crate::plans::DropFileFormatPlan;
use crate::plans::DropRolePlan;
use crate::plans::DropStagePlan;
use crate::plans::DropUDFPlan;
use crate::plans::DropUserPlan;
use crate::plans::MaterializedCte;
use crate::plans::Plan;
Expand Down Expand Up @@ -479,10 +478,7 @@ impl<'a> Binder {
Statement::DropUDF {
if_exists,
udf_name,
} => Plan::DropUDF(Box::new(DropUDFPlan {
if_exists: *if_exists,
udf: udf_name.to_string(),
})),
} => self.bind_drop_udf(*if_exists, udf_name).await?,
Statement::Call(stmt) => self.bind_call(bind_context, stmt).await?,

Statement::Presign(stmt) => self.bind_presign(bind_context, stmt).await?,
Expand Down
21 changes: 18 additions & 3 deletions src/query/sql/src/planner/binder/udf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ use databend_common_meta_app::principal::UDFScript;
use databend_common_meta_app::principal::UDFServer;
use databend_common_meta_app::principal::UserDefinedFunction;

use crate::normalize_identifier;
use crate::planner::resolve_type_name;
use crate::planner::udf_validator::UDFValidator;
use crate::plans::AlterUDFPlan;
use crate::plans::CreateUDFPlan;
use crate::plans::DropUDFPlan;
use crate::plans::Plan;
use crate::Binder;

Expand All @@ -42,13 +44,14 @@ impl Binder {
udf_description: &Option<String>,
udf_definition: &UDFDefinition,
) -> Result<UserDefinedFunction> {
let name = normalize_identifier(udf_name, &self.name_resolution_ctx).to_string();
match udf_definition {
UDFDefinition::LambdaUDF {
parameters,
definition,
} => {
let mut validator = UDFValidator {
name: udf_name.to_string(),
name,
parameters: parameters.iter().map(|v| v.to_string()).collect(),
..Default::default()
};
Expand Down Expand Up @@ -107,7 +110,7 @@ impl Binder {
.await?;

Ok(UserDefinedFunction {
name: udf_name.to_string(),
name,
description: udf_description.clone().unwrap_or_default(),
definition: PlanUDFDefinition::UDFServer(UDFServer {
address: address.clone(),
Expand Down Expand Up @@ -152,7 +155,7 @@ impl Binder {
}

Ok(UserDefinedFunction {
name: udf_name.to_string(),
name,
description: udf_description.clone().unwrap_or_default(),
definition: PlanUDFDefinition::UDFScript(UDFScript {
code: code.clone(),
Expand Down Expand Up @@ -190,4 +193,16 @@ impl Binder {
.await?;
Ok(Plan::AlterUDF(Box::new(AlterUDFPlan { udf })))
}

pub(in crate::planner::binder) async fn bind_drop_udf(
&mut self,
if_exists: bool,
udf_name: &Identifier,
) -> Result<Plan> {
let name = normalize_identifier(udf_name, &self.name_resolution_ctx).to_string();
Ok(Plan::DropUDF(Box::new(DropUDFPlan {
if_exists,
udf: name,
})))
}
}
54 changes: 54 additions & 0 deletions tests/sqllogictests/suites/base/05_ddl/05_0010_ddl_create_udf.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,57 @@ CREATE OR REPLACE FUNCTION isnotempty_test_replace AS(p) -> not(is_null(p)) DES

statement ok
DROP FUNCTION IF EXISTS isnotempty_test_replace;

statement ok
create or replace function SOME_NAME as (x) -> x * x

query I
SELECT SOME_NAME(2)
----
4

query I
SELECT some_name(2)
----
4

statement ok
alter function SOME_NAME as (x) -> x * x + 1

query I
SELECT SOME_NAME(2)
----
5

query I
SELECT some_name(2)
----
5

statement ok
drop function SOME_NAME

statement ok
create or replace function `SOME_NAME` as (x) -> x * x

query I
SELECT `SOME_NAME`(2)
----
4

statement error 1008
SELECT SOME_NAME(2)

statement error 1008
SELECT `some_name`(2)

statement ok
alter function `SOME_NAME` as (x) -> x * x + 1

query I
SELECT `SOME_NAME`(2)
----
5

statement ok
drop function `SOME_NAME`

0 comments on commit e5e2d5c

Please sign in to comment.