Skip to content

Commit ef12877

Browse files
committed
fix explain ast for invalid query
1 parent 985d6f8 commit ef12877

File tree

7 files changed

+63
-18
lines changed

7 files changed

+63
-18
lines changed

src/query/service/src/interpreters/interpreter_factory.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ impl InterpreterFactory {
3939
pub async fn get(ctx: Arc<QueryContext>, plan: PlanNode) -> Result<Arc<dyn Interpreter>> {
4040
// Check the access permission.
4141
let access_checker = Accessor::create(ctx.clone());
42-
access_checker
43-
.check(&plan)
44-
.await
45-
.map(|e| error!("Access.denied(v1): {:?}", e))?;
42+
access_checker.check(&plan).await.map_err(|e| {
43+
error!("Access.denied(v1): {:?}", e);
44+
e
45+
})?;
4646

4747
let inner = Self::create_interpreter(ctx.clone(), &plan)?;
4848
let query_kind = plan.name().to_string();

src/query/service/src/interpreters/interpreter_factory_v2.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
use std::sync::Arc;
1616

17+
use common_ast::ast::ExplainKind;
1718
use common_exception::Result;
1819
use tracing::error;
1920

@@ -46,10 +47,10 @@ impl InterpreterFactoryV2 {
4647
pub async fn get(ctx: Arc<QueryContext>, plan: &Plan) -> Result<InterpreterPtr> {
4748
// Check the access permission.
4849
let access_checker = Accessor::create(ctx.clone());
49-
access_checker
50-
.check_new(plan)
51-
.await
52-
.map(|e| error!("Access.denied(v2): {:?}", e))?;
50+
access_checker.check_new(plan).await.map_err(|e| {
51+
error!("Access.denied(v2): {:?}", e);
52+
e
53+
})?;
5354

5455
let inner = InterpreterFactoryV2::create_interpreter(ctx.clone(), plan)?;
5556

@@ -78,6 +79,20 @@ impl InterpreterFactoryV2 {
7879
*plan.clone(),
7980
kind.clone(),
8081
)?)),
82+
Plan::ExplainAst { formatted_string } => {
83+
Ok(Arc::new(ExplainInterpreterV2::try_create(
84+
ctx,
85+
plan.clone(),
86+
ExplainKind::Ast(formatted_string.clone()),
87+
)?))
88+
}
89+
Plan::ExplainSyntax { formatted_sql } => {
90+
Ok(Arc::new(ExplainInterpreterV2::try_create(
91+
ctx,
92+
plan.clone(),
93+
ExplainKind::Syntax(formatted_sql.clone()),
94+
)?))
95+
}
8196

8297
Plan::Call(plan) => Ok(Arc::new(CallInterpreter::try_create(ctx, *plan.clone())?)),
8398

src/query/service/src/servers/mysql/mysql_interactive_worker.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ fn has_result_set_by_plan(plan: &Plan) -> bool {
6464
plan,
6565
Plan::Query { .. }
6666
| Plan::Explain { .. }
67+
| Plan::ExplainAst { .. }
68+
| Plan::ExplainSyntax { .. }
6769
| Plan::Call(_)
6870
| Plan::ShowCreateDatabase(_)
6971
| Plan::ShowCreateTable(_)
@@ -221,10 +223,10 @@ impl<W: AsyncWrite + Send + Sync + Unpin> AsyncMysqlShim<W> for InteractiveWorke
221223
let mut writer = DFQueryResultWriter::create(writer);
222224

223225
let instant = Instant::now();
224-
let blocks = self.base.do_query(query).await;
226+
let query_result = self.base.do_query(query).await;
225227

226228
let format = self.base.session.get_format_settings()?;
227-
let mut write_result = writer.write(blocks, &format).await;
229+
let mut write_result = writer.write(query_result, &format).await;
228230

229231
if let Err(cause) = write_result {
230232
let suffix = format!("(while in query {})", query);

src/query/service/src/sql/planner/binder/mod.rs

+8-4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::sync::Arc;
1616

1717
pub use aggregate::AggregateInfo;
1818
pub use bind_context::*;
19+
use common_ast::ast::ExplainKind;
1920
use common_ast::ast::Statement;
2021
use common_ast::parser::parse_sql;
2122
use common_ast::parser::tokenize_sql;
@@ -118,10 +119,13 @@ impl<'a> Binder {
118119
}
119120
}
120121

121-
Statement::Explain { query, kind } => Plan::Explain {
122-
kind: kind.clone(),
123-
plan: Box::new(self.bind_statement(bind_context, query).await?),
124-
},
122+
Statement::Explain { query, kind } => {
123+
match kind {
124+
ExplainKind::Ast(formatted_stmt) => Plan::ExplainAst { formatted_string: formatted_stmt.clone() },
125+
ExplainKind::Syntax(formatted_sql) => Plan::ExplainSyntax { formatted_sql: formatted_sql.clone() },
126+
_ => Plan::Explain { kind: kind.clone(), plan: Box::new(self.bind_statement(bind_context, query).await?) },
127+
}
128+
}
125129

126130
Statement::ShowFunctions { limit } => {
127131
self.bind_show_functions(bind_context, limit).await?

src/query/service/src/sql/planner/format/display_plan.rs

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ impl Plan {
2626
let result = plan.format_indent()?;
2727
Ok(format!("{:?}:\n{}", kind, result))
2828
}
29+
Plan::ExplainAst { .. } => Ok("ExplainAst".to_string()),
30+
Plan::ExplainSyntax { .. } => Ok("ExplainSyntax".to_string()),
2931

3032
Plan::Copy(plan) => Ok(format!("{:?}", plan)),
3133

src/query/service/src/sql/planner/plans/mod.rs

+11-4
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ use common_datavalues::DataField;
4343
use common_datavalues::DataSchema;
4444
use common_datavalues::DataSchemaRef;
4545
use common_datavalues::DataSchemaRefExt;
46-
use common_datavalues::ToDataType;
47-
use common_datavalues::Vu8;
46+
use common_datavalues::StringType;
4847
use common_legacy_planners::AlterTableClusterKeyPlan;
4948
use common_legacy_planners::AlterUserPlan;
5049
use common_legacy_planners::AlterUserUDFPlan;
@@ -133,6 +132,12 @@ pub enum Plan {
133132
kind: ExplainKind,
134133
plan: Box<Plan>,
135134
},
135+
ExplainAst {
136+
formatted_string: String,
137+
},
138+
ExplainSyntax {
139+
formatted_sql: String,
140+
},
136141

137142
// Copy
138143
Copy(Box<CopyPlanV2>),
@@ -293,6 +298,8 @@ impl Display for Plan {
293298
Plan::ShowShares(_) => write!(f, "ShowShares"),
294299
Plan::ShowObjectGrantPrivileges(_) => write!(f, "ShowObjectGrantPrivileges"),
295300
Plan::ShowGrantTenantsOfShare(_) => write!(f, "ShowGrantTenantsOfShare"),
301+
Plan::ExplainAst { .. } => write!(f, "ExplainAst"),
302+
Plan::ExplainSyntax { .. } => write!(f, "ExplainSyntax"),
296303
}
297304
}
298305
}
@@ -307,8 +314,8 @@ impl Plan {
307314
bind_context,
308315
..
309316
} => bind_context.output_schema(),
310-
Plan::Explain { kind: _, plan: _ } => {
311-
DataSchemaRefExt::create(vec![DataField::new("explain", Vu8::to_data_type())])
317+
Plan::Explain { .. } | Plan::ExplainAst { .. } | Plan::ExplainSyntax { .. } => {
318+
DataSchemaRefExt::create(vec![DataField::new("explain", StringType::new_impl())])
312319
}
313320
Plan::Copy(_) => Arc::new(DataSchema::empty()),
314321
Plan::ShowCreateDatabase(plan) => plan.schema(),

tests/logictest/suites/mode/standalone/explain/explain.test

+15
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,21 @@ CreateUser (children 3)
498498
├── AuthType sha256_password
499499
└── Password "new_password"
500500

501+
statement query T
502+
explain ast select unknown_table.a + 1 from unknown_table1;
503+
504+
----
505+
Query (children 1)
506+
└── QueryBody (children 1)
507+
└── SelectQuery (children 2)
508+
├── SelectList (children 1)
509+
│ └── Target (children 1)
510+
│ └── Function + (children 2)
511+
│ ├── ColumnIdentifier unknown_table.a
512+
│ └── Literal Integer(1)
513+
└── TableList (children 1)
514+
└── TableIdentifier unknown_table1
515+
501516
statement ok
502517
drop table t1;
503518

0 commit comments

Comments
 (0)