Skip to content

Commit 54e86cf

Browse files
authored
Use parsed AST instead of double-parsing SELECT stmt (#527)
1 parent cab4685 commit 54e86cf

File tree

4 files changed

+19
-16
lines changed

4 files changed

+19
-16
lines changed

pgdog/src/frontend/router/parser/query/explain.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@ use super::*;
33
impl QueryParser {
44
pub(super) fn explain(
55
&mut self,
6+
ast: &pg_query::ParseResult,
67
stmt: &ExplainStmt,
78
context: &mut QueryParserContext,
89
) -> Result<Command, Error> {
910
let query = stmt.query.as_ref().ok_or(Error::EmptyQuery)?;
1011
let node = query.node.as_ref().ok_or(Error::EmptyQuery)?;
1112

1213
match node {
13-
NodeEnum::SelectStmt(ref stmt) => self.select(stmt, context),
14+
NodeEnum::SelectStmt(ref stmt) => self.select(ast, stmt, context),
1415
NodeEnum::InsertStmt(ref stmt) => Self::insert(stmt, context),
1516
NodeEnum::UpdateStmt(ref stmt) => Self::update(stmt, context),
1617
NodeEnum::DeleteStmt(ref stmt) => Self::delete(stmt, context),

pgdog/src/frontend/router/parser/query/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ impl QueryParser {
212212
return Ok(Command::Deallocate);
213213
}
214214
// SELECT statements.
215-
Some(NodeEnum::SelectStmt(ref stmt)) => self.select(stmt, context),
215+
Some(NodeEnum::SelectStmt(ref stmt)) => self.select(statement.ast(), stmt, context),
216216
// COPY statements.
217217
Some(NodeEnum::CopyStmt(ref stmt)) => Self::copy(stmt, context),
218218
// INSERT statements.
@@ -258,7 +258,7 @@ impl QueryParser {
258258
return Ok(Command::Unlisten(stmt.conditionname.clone()));
259259
}
260260

261-
Some(NodeEnum::ExplainStmt(ref stmt)) => self.explain(stmt, context),
261+
Some(NodeEnum::ExplainStmt(ref stmt)) => self.explain(statement.ast(), stmt, context),
262262

263263
// VACUUM.
264264
Some(NodeEnum::VacuumRelation(_)) | Some(NodeEnum::VacuumStmt(_)) => {

pgdog/src/frontend/router/parser/query/select.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ impl QueryParser {
1212
///
1313
pub(super) fn select(
1414
&mut self,
15+
ast: &pg_query::ParseResult,
1516
stmt: &SelectStmt,
1617
context: &mut QueryParserContext,
1718
) -> Result<Command, Error> {
@@ -93,8 +94,11 @@ impl QueryParser {
9394
// Only rewrite if query is cross-shard.
9495
if query.is_cross_shard() && context.shards > 1 {
9596
if let Some(buffered_query) = context.router_context.query.as_ref() {
96-
let rewrite =
97-
RewriteEngine::new().rewrite_select(buffered_query.query(), query.aggregate());
97+
let rewrite = RewriteEngine::new().rewrite_select(
98+
ast,
99+
buffered_query.query(),
100+
query.aggregate(),
101+
);
98102
if !rewrite.plan.is_noop() {
99103
if let BufferedQuery::Prepared(parse) = buffered_query {
100104
let name = parse.name().to_owned();

pgdog/src/frontend/router/parser/rewrite_engine.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use super::{Aggregate, AggregateFunction, HelperMapping, RewriteOutput, RewritePlan};
22
use pg_query::protobuf::{FuncCall, Node, ResTarget, String as PgString};
33
use pg_query::{NodeEnum, ParseResult};
4-
use tracing::debug;
54

65
/// Query rewrite engine. Currently supports injecting helper aggregates for AVG.
76
#[derive(Default)]
@@ -13,19 +12,18 @@ impl RewriteEngine {
1312
}
1413

1514
/// Rewrite a SELECT query, adding helper aggregates when necessary.
16-
pub fn rewrite_select(&self, sql: &str, aggregate: &Aggregate) -> RewriteOutput {
17-
match pg_query::parse(sql) {
18-
Ok(parsed) => self.rewrite_parsed(parsed, aggregate, sql),
19-
Err(err) => {
20-
debug!("rewrite failed to parse SELECT: {}", err);
21-
RewriteOutput::new(sql.to_string(), RewritePlan::new())
22-
}
23-
}
15+
pub fn rewrite_select(
16+
&self,
17+
ast: &ParseResult,
18+
sql: &str,
19+
aggregate: &Aggregate,
20+
) -> RewriteOutput {
21+
self.rewrite_parsed(ast, aggregate, sql)
2422
}
2523

2624
fn rewrite_parsed(
2725
&self,
28-
parsed: ParseResult,
26+
parsed: &ParseResult,
2927
aggregate: &Aggregate,
3028
original_sql: &str,
3129
) -> RewriteOutput {
@@ -171,7 +169,7 @@ mod tests {
171169
None => panic!("empty"),
172170
};
173171
let aggregate = Aggregate::parse(stmt).unwrap();
174-
RewriteEngine::new().rewrite_select(sql, &aggregate)
172+
RewriteEngine::new().rewrite_select(&ast, sql, &aggregate)
175173
}
176174

177175
#[test]

0 commit comments

Comments
 (0)