NGA-TRAN commented on code in PR #4490:
URL: https://github.com/apache/arrow-datafusion/pull/4490#discussion_r1039641430
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -108,6 +108,8 @@ pub enum LogicalPlan {
Distinct(Distinct),
/// Set a Variable
SetVariable(SetVariable),
+ /// Prepare a statement
+ Prepare(Prepare),
Review Comment:
I think so. It will be cleaner unless you say otherwise
##########
datafusion/expr/src/logical_plan/builder.rs:
##########
@@ -119,6 +121,7 @@ impl LogicalPlanBuilder {
/// The column names are not specified by the SQL standard and different
database systems do it differently,
/// so it's usually better to override the default names with a table
alias list.
pub fn values(mut values: Vec<Vec<Expr>>) -> Result<Self> {
+ // todo: hanlde for Placeholder expr
Review Comment:
Thanks @alamb. Let me have a quick look to see how to implement this and get
back to you with questions if any
##########
datafusion/sql/src/planner.rs:
##########
@@ -5992,6 +6011,20 @@ mod tests {
quick_test(sql, expected);
}
+ // TODO: will ad more tests to cover maby other cases
+ #[test]
+ fn test_prepare_statement_to_plan() {
+ let sql = "PREPARE my_plan(INT) AS SELECT id, age FROM person WHERE
age = $1";
+ //let statements = DFParser::parse_sql(sql).unwrap();
+
+ let expected = "Prepare: \"my_plan\" [Int32] \
+ \n Projection: person.id, person.age\
+ \n Filter: person.age = $1\
+ \n TableScan: person";
+
+ quick_test(sql, expected);
+ }
+
Review Comment:
Yes, I have planned to refactor `quick_test` or write a similar function to
verify the placeholders and their types
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
