Skip to content

Commit 3f26300

Browse files
refactor: receive parsed statement in planner rather than raw sql (#753)
Please be sure to look over the pull request guidelines here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md#submit-pr. # Please go through the following checklist - [ ] The PR title and commit messages adhere to guidelines here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/CONTRIBUTING.md. In particular `!` is used if and only if at least one breaking change has been introduced. - [ ] I have run the ci check script with `source scripts/run_ci_checks.sh`. - [ ] I have run the clean commit check script with `source scripts/check_commits.sh`, and the commit history is certified to follow clean commit guidelines as described here: https://github.com/spaceandtimelabs/sxt-proof-of-sql/blob/main/COMMIT_GUIDELINES.md - [ ] The latest changes from `main` have been incorporated to this PR by simple rebase if possible, if not, then conflicts are resolved appropriately. # Rationale for this change In order to avoid parsing twice, we want the planner to use a parsed statement, rather than raw sql, to produce the `DynProofPlan`. # What changes are included in this PR? Modifying `conversion` functions to use `Statement`s rather than raw sql. # Are these changes tested? Yes
2 parents af56f22 + f6b022a commit 3f26300

File tree

2 files changed

+14
-15
lines changed

2 files changed

+14
-15
lines changed

crates/proof-of-sql-planner/src/conversion.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@ use proof_of_sql::{
1616
base::database::{ParseError, TableRef},
1717
sql::proof_plans::DynProofPlan,
1818
};
19-
use sqlparser::{
20-
ast::{visit_relations, Statement},
21-
dialect::GenericDialect,
22-
parser::Parser,
23-
};
19+
use sqlparser::ast::{visit_relations, Statement};
2420
use std::ops::ControlFlow;
2521

2622
/// Get [`Optimizer`]
@@ -51,7 +47,7 @@ pub fn optimizer() -> Optimizer {
5147
/// 4. Optimize the `LogicalPlan` using `Optimizer`
5248
/// 5. Convert the optimized `LogicalPlan` into a Proof of SQL plan
5349
fn sql_to_posql_plans<S, T, F>(
54-
sql: &str,
50+
statements: &[Statement],
5551
context_provider: &S,
5652
schemas: &IndexMap<TableReference, DFSchema>,
5753
config: &ConfigOptions,
@@ -62,9 +58,8 @@ where
6258
F: Fn(&LogicalPlan, &IndexMap<TableReference, DFSchema>) -> PlannerResult<T>,
6359
{
6460
// 1. Parse the SQL query into AST using sqlparser
65-
let dialect = GenericDialect {};
66-
let asts = Parser::parse_sql(&dialect, sql)?;
67-
asts.iter()
61+
statements
62+
.iter()
6863
.map(|ast| -> PlannerResult<T> {
6964
// 2. Convert the AST into a `LogicalPlan` using `SqlToRel`
7065
let raw_logical_plan =
@@ -88,13 +83,13 @@ where
8883
///
8984
/// See `sql_to_posql_plans` for more details
9085
pub fn sql_to_proof_plans<S: ContextProvider>(
91-
sql: &str,
86+
statements: &[Statement],
9287
context_provider: &S,
9388
schemas: &IndexMap<TableReference, DFSchema>,
9489
config: &ConfigOptions,
9590
) -> PlannerResult<Vec<DynProofPlan>> {
9691
sql_to_posql_plans(
97-
sql,
92+
statements,
9893
context_provider,
9994
schemas,
10095
config,
@@ -106,13 +101,13 @@ pub fn sql_to_proof_plans<S: ContextProvider>(
106101
///
107102
/// See `sql_to_posql_plans` for more details
108103
pub fn sql_to_proof_plans_with_postprocessing<S: ContextProvider>(
109-
sql: &str,
104+
statements: &[Statement],
110105
context_provider: &S,
111106
schemas: &IndexMap<TableReference, DFSchema>,
112107
config: &ConfigOptions,
113108
) -> PlannerResult<Vec<ProofPlanWithPostprocessing>> {
114109
sql_to_posql_plans(
115-
sql,
110+
statements,
116111
context_provider,
117112
schemas,
118113
config,

crates/proof-of-sql-planner/tests/e2e_tests.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use proof_of_sql_planner::{
2121
column_fields_to_schema, postprocessing::PostprocessingStep, sql_to_proof_plans,
2222
sql_to_proof_plans_with_postprocessing, PlannerResult, PoSqlContextProvider,
2323
};
24+
use sqlparser::{dialect::GenericDialect, parser::Parser};
2425

2526
/// Get a new `TableTestAccessor` with the provided tables
2627
fn new_test_accessor<'a, CP: CommitmentEvaluationProof>(
@@ -69,7 +70,8 @@ fn posql_end_to_end_test<'a, CP: CommitmentEvaluationProof>(
6970
let schemas = get_schemas::<CP>(&tables).unwrap();
7071
let context_provider = PoSqlContextProvider::new(tables);
7172
let config = ConfigOptions::default();
72-
let plans = sql_to_proof_plans(sql, &context_provider, &schemas, &config).unwrap();
73+
let statements = Parser::parse_sql(&GenericDialect {}, sql).unwrap();
74+
let plans = sql_to_proof_plans(&statements, &context_provider, &schemas, &config).unwrap();
7375
// Prove and verify the plans
7476
for (plan, expected) in plans.iter().zip(expected_results.iter()) {
7577
let res = VerifiableQueryResult::<CP>::new(plan, &accessor, &prover_setup, params).unwrap();
@@ -96,8 +98,10 @@ fn posql_end_to_end_test_with_postprocessing<'a, CP: CommitmentEvaluationProof>(
9698
let schemas = get_schemas::<CP>(&tables).unwrap();
9799
let context_provider = PoSqlContextProvider::new(tables);
98100
let config = ConfigOptions::default();
101+
let statements = Parser::parse_sql(&GenericDialect {}, sql).unwrap();
99102
let plan_with_postprocessings =
100-
sql_to_proof_plans_with_postprocessing(sql, &context_provider, &schemas, &config).unwrap();
103+
sql_to_proof_plans_with_postprocessing(&statements, &context_provider, &schemas, &config)
104+
.unwrap();
101105
for (plan_with_postprocessing, expected) in plan_with_postprocessings
102106
.iter()
103107
.zip(expected_results.iter())

0 commit comments

Comments
 (0)