Skip to content

Commit 9f5783e

Browse files
TCeasonTCeason
committed
refactor(ast): use Vec<TypeName> in ProcedureIdentity instead of String
This change improves type safety by preserving TypeName information from the parser to the binder, avoiding unnecessary string round-trip conversions when handling complex types like Decimal(4, 2).
1 parent 681dead commit 9f5783e

File tree

6 files changed

+211
-107
lines changed

6 files changed

+211
-107
lines changed

‎src/meta/app/src/principal/procedure_identity.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ impl KeyCodec for ProcedureIdentity {
6464

6565
impl From<databend_common_ast::ast::ProcedureIdentity> for ProcedureIdentity {
6666
fn from(procedure: databend_common_ast::ast::ProcedureIdentity) -> Self {
67-
ProcedureIdentity::new(procedure.name, procedure.args_type)
67+
let args_type_str = procedure
68+
.args_type
69+
.iter()
70+
.map(|t| t.to_string())
71+
.collect::<Vec<_>>()
72+
.join(",");
73+
ProcedureIdentity::new(procedure.name, args_type_str)
6874
}
6975
}

‎src/query/ast/src/ast/statements/procedure.rs‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,41 @@ impl Display for ProcedureLanguage {
6565
}
6666
}
6767

68-
#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
68+
#[derive(Clone, PartialEq, Drive, DriveMut)]
6969
pub struct ProcedureIdentity {
7070
pub name: String,
71-
pub args_type: String,
71+
pub args_type: Vec<TypeName>,
72+
}
73+
74+
impl std::fmt::Debug for ProcedureIdentity {
75+
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
76+
f.debug_struct("ProcedureIdentity")
77+
.field("name", &self.name)
78+
.field(
79+
"args_type",
80+
&self
81+
.args_type
82+
.iter()
83+
.map(|t| t.to_string())
84+
.collect::<Vec<_>>()
85+
.join(","),
86+
)
87+
.finish()
88+
}
7289
}
7390

7491
impl Display for ProcedureIdentity {
7592
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
76-
write!(f, "{}({})", &self.name, &self.args_type,)
93+
write!(
94+
f,
95+
"{}({})",
96+
&self.name,
97+
self.args_type
98+
.iter()
99+
.map(|t| t.to_string())
100+
.collect::<Vec<_>>()
101+
.join(",")
102+
)
77103
}
78104
}
79105

‎src/query/ast/src/ast/statements/user.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ impl Display for AlterUserStmt {
104104
}
105105
}
106106

107-
#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
107+
#[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
108108
pub struct GrantStmt {
109109
pub source: AccountMgrSource,
110110
pub principal: PrincipalIdentity,
@@ -120,7 +120,7 @@ impl Display for GrantStmt {
120120
}
121121
}
122122

123-
#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
123+
#[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
124124
pub struct RevokeStmt {
125125
pub source: AccountMgrSource,
126126
pub principal: PrincipalIdentity,
@@ -205,7 +205,7 @@ impl Display for ShowObjectPrivilegesStmt {
205205
}
206206
}
207207

208-
#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
208+
#[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
209209
pub enum AccountMgrSource {
210210
Role {
211211
role: String,
@@ -239,7 +239,7 @@ impl Display for AccountMgrSource {
239239
}
240240
}
241241

242-
#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
242+
#[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
243243
pub enum AccountMgrLevel {
244244
Global,
245245
Database(Option<String>),

‎src/query/ast/src/parser/statement.rs‎

Lines changed: 8 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2489,12 +2489,9 @@ pub fn statement_body(i: Input) -> IResult<Statement> {
24892489
let name = ProcedureIdentity {
24902490
name: name.to_string(),
24912491
args_type: if let Some(args) = &args {
2492-
args.iter()
2493-
.map(|arg| arg.data_type.to_string())
2494-
.collect::<Vec<String>>()
2495-
.join(",")
2492+
args.iter().map(|arg| arg.data_type.clone()).collect()
24962493
} else {
2497-
"".to_string()
2494+
vec![]
24982495
},
24992496
};
25002497
let stmt = CreateProcedureStmt {
@@ -2558,14 +2555,7 @@ pub fn statement_body(i: Input) -> IResult<Statement> {
25582555
if_exists: opt_if_exists.is_some(),
25592556
name: ProcedureIdentity {
25602557
name: name.to_string(),
2561-
args_type: if args.is_empty() {
2562-
"".to_string()
2563-
} else {
2564-
args.iter()
2565-
.map(|arg| arg.to_string())
2566-
.collect::<Vec<String>>()
2567-
.join(",")
2568-
},
2558+
args_type: args,
25692559
},
25702560
})
25712561
},
@@ -2579,14 +2569,7 @@ pub fn statement_body(i: Input) -> IResult<Statement> {
25792569
Statement::DescProcedure(DescProcedureStmt {
25802570
name: ProcedureIdentity {
25812571
name: name.to_string(),
2582-
args_type: if args.is_empty() {
2583-
"".to_string()
2584-
} else {
2585-
args.iter()
2586-
.map(|arg| arg.to_string())
2587-
.collect::<Vec<String>>()
2588-
.join(",")
2589-
},
2572+
args_type: args,
25902573
},
25912574
})
25922575
},
@@ -3636,14 +3619,7 @@ pub fn grant_source(i: Input) -> IResult<AccountMgrSource> {
36363619
privileges: vec![UserPrivilegeType::AccessProcedure],
36373620
level: AccountMgrLevel::Procedure(ProcedureIdentity {
36383621
name: name.to_string(),
3639-
args_type: if args.is_empty() {
3640-
"".to_string()
3641-
} else {
3642-
args.iter()
3643-
.map(|arg| arg.to_string())
3644-
.collect::<Vec<String>>()
3645-
.join(",")
3646-
},
3622+
args_type: args,
36473623
}),
36483624
},
36493625
);
@@ -3656,14 +3632,7 @@ pub fn grant_source(i: Input) -> IResult<AccountMgrSource> {
36563632
privileges: vec![UserPrivilegeType::AccessProcedure],
36573633
level: AccountMgrLevel::Procedure(ProcedureIdentity {
36583634
name: name.to_string(),
3659-
args_type: if args.is_empty() {
3660-
"".to_string()
3661-
} else {
3662-
args.iter()
3663-
.map(|arg| arg.to_string())
3664-
.collect::<Vec<String>>()
3665-
.join(",")
3666-
},
3635+
args_type: args,
36673636
}),
36683637
},
36693638
);
@@ -3833,14 +3802,7 @@ pub fn on_object_name(i: Input) -> IResult<GrantObjectName> {
38333802
|(_, name, args)| {
38343803
GrantObjectName::Procedure(ProcedureIdentity {
38353804
name: name.to_string(),
3836-
args_type: if args.is_empty() {
3837-
"".to_string()
3838-
} else {
3839-
args.iter()
3840-
.map(|arg| arg.to_string())
3841-
.collect::<Vec<String>>()
3842-
.join(",")
3843-
},
3805+
args_type: args,
38443806
})
38453807
},
38463808
);
@@ -3979,14 +3941,7 @@ pub fn grant_ownership_level(i: Input) -> IResult<AccountMgrLevel> {
39793941
|(_, name, args)| {
39803942
let name = ProcedureIdentity {
39813943
name: name.to_string(),
3982-
args_type: if args.is_empty() {
3983-
"".to_string()
3984-
} else {
3985-
args.iter()
3986-
.map(|arg| arg.to_string())
3987-
.collect::<Vec<String>>()
3988-
.join(",")
3989-
},
3944+
args_type: args,
39903945
};
39913946
AccountMgrLevel::Procedure(name)
39923947
},

‎src/query/sql/src/planner/binder/ddl/procedure.rs‎

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ use crate::plans::Plan;
4949
use crate::plans::RewriteKind;
5050
use crate::plans::SubqueryType;
5151
use crate::resolve_type_name;
52-
use crate::resolve_type_name_by_str;
5352
use crate::BindContext;
5453
use crate::Binder;
5554
use crate::ScalarExpr;
@@ -266,53 +265,20 @@ fn generate_procedure_name_ident(
266265
return Ok(ProcedureNameIdent::new(tenant, name.clone().into()));
267266
}
268267

269-
let args = split_procedure_arg_types(&name.args_type)?;
270-
let mut args_type = vec![];
271-
for arg in args {
272-
args_type.push(DataType::from(&resolve_type_name_by_str(&arg, true)?));
268+
let mut args_data_type = Vec::with_capacity(name.args_type.len());
269+
for type_name in &name.args_type {
270+
args_data_type.push(DataType::from(&resolve_type_name(type_name, true)?));
273271
}
274-
let new_name = databend_common_ast::ast::ProcedureIdentity {
275-
name: name.name.to_string(),
276-
args_type: args_type
277-
.iter()
278-
.map(|arg| arg.to_string())
279-
.collect::<Vec<String>>()
280-
.join(","),
281-
};
272+
273+
// Convert normalized DataType back to string for storage
274+
let args_type_str = args_data_type
275+
.iter()
276+
.map(|dt| dt.to_string())
277+
.collect::<Vec<_>>()
278+
.join(",");
279+
282280
Ok(ProcedureNameIdent::new(
283281
tenant,
284-
ProcedureIdentity::from(new_name),
282+
ProcedureIdentity::new(name.name.clone(), args_type_str),
285283
))
286284
}
287-
288-
fn split_procedure_arg_types(raw: &str) -> Result<Vec<String>> {
289-
let mut parts = Vec::new();
290-
let mut depth = 0;
291-
let mut start = 0;
292-
293-
for (i, c) in raw.char_indices() {
294-
match c {
295-
'(' => depth += 1,
296-
')' => depth -= 1,
297-
',' if depth == 0 => {
298-
let arg = raw[start..i].trim();
299-
if !arg.is_empty() {
300-
parts.push(arg.to_string());
301-
}
302-
start = i + 1;
303-
}
304-
_ => {}
305-
}
306-
}
307-
308-
if depth != 0 {
309-
return Err(ErrorCode::SyntaxException("unmatched parentheses"));
310-
}
311-
312-
let last = raw[start..].trim();
313-
if !last.is_empty() {
314-
parts.push(last.to_string());
315-
}
316-
317-
Ok(parts)
318-
}

0 commit comments

Comments
 (0)