Skip to content

Commit

Permalink
fix: address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
iajoiner committed Dec 4, 2024
1 parent 975fec1 commit e4bb220
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 42 deletions.
3 changes: 3 additions & 0 deletions crates/proof-of-sql/src/base/proof/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@ pub enum ProofError {
#[snafu(display("Verification error: {error}"))]
/// This error occurs when a proof failed to verify.
VerificationError { error: &'static str },
/// Unsupported error
#[snafu(display("Unsupported error: {error}"))]
UnsupportedError { error: &'static str },
}
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl ProverEvaluate for FilterExec {
}

#[allow(clippy::unnecessary_wraps, clippy::too_many_arguments)]
pub(crate) fn verify_filter<S: Scalar>(
pub(super) fn verify_filter<S: Scalar>(
builder: &mut VerificationBuilder<S>,
alpha: S,
beta: S,
Expand Down
2 changes: 1 addition & 1 deletion crates/proof-of-sql/src/sql/proof_plans/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ mod projection_exec_test;
pub(crate) mod test_utility;

mod filter_exec;
pub(super) use filter_exec::FilterExec;
#[cfg(test)]
pub(crate) use filter_exec::OstensibleFilterExec;
pub(crate) use filter_exec::{prove_filter, verify_filter, FilterExec};
#[cfg(all(test, feature = "blitzar"))]
mod filter_exec_test;
#[cfg(all(test, feature = "blitzar"))]
Expand Down
40 changes: 23 additions & 17 deletions crates/proof-of-sql/src/sql/proof_plans/slice_exec.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use super::{prove_filter, verify_filter, DynProofPlan};
use super::{
filter_exec::{prove_filter, verify_filter},
DynProofPlan,
};
use crate::{
base::{
database::{
Expand All @@ -16,7 +19,8 @@ use crate::{
};
use alloc::{boxed::Box, vec, vec::Vec};
use bumpalo::Bump;
use core::iter::repeat_with;
use core::iter::{repeat, repeat_with};
use itertools::repeat_n;
use serde::{Deserialize, Serialize};

/// `ProofPlan` for queries of the form
Expand All @@ -32,12 +36,11 @@ pub struct SliceExec {

/// Get the boolean slice selection from the number of rows, skip and fetch
fn get_slice_select(num_rows: usize, skip: usize, fetch: Option<usize>) -> Vec<bool> {
if let Some(fetch) = fetch {
let end = skip + fetch;
(0..num_rows).map(|i| i >= skip && i < end).collect()
} else {
(0..num_rows).map(|i| i >= skip).collect()
}
repeat_n(false, skip)
.chain(repeat_n(true, fetch.unwrap_or(num_rows)))
.chain(repeat(false))
.take(num_rows)
.collect()
}

impl SliceExec {
Expand Down Expand Up @@ -71,7 +74,12 @@ where
one_eval_map: &IndexMap<TableRef, S>,
) -> Result<TableEvaluation<S>, ProofError> {
// 1. columns
// TODO: Make sure `GroupByExec` as self.input is supported
// We do not support `GroupByExec` as input for now
if let DynProofPlan::GroupBy(_) = *self.input {
return Err(ProofError::UnsupportedError {
error: "GroupByExec as input for another plan is not supported",
});
}
let input_table_eval =
self.input
.verifier_evaluate(builder, accessor, None, one_eval_map)?;
Expand Down Expand Up @@ -119,14 +127,16 @@ where
}

impl ProverEvaluate for SliceExec {
#[tracing::instrument(name = "SliceExec::result_evaluate", level = "debug", skip_all)]
fn result_evaluate<'a, S: Scalar>(
#[tracing::instrument(name = "SliceExec::first_round_evaluate", level = "debug", skip_all)]
fn first_round_evaluate<'a, S: Scalar>(
&self,
builder: &mut FirstRoundBuilder,
alloc: &'a Bump,
table_map: &IndexMap<TableRef, Table<'a, S>>,
) -> (Table<'a, S>, Vec<usize>) {
// 1. columns
let (input, input_one_eval_lengths) = self.input.result_evaluate(alloc, table_map);
let (input, input_one_eval_lengths) =
self.input.first_round_evaluate(builder, alloc, table_map);
let input_length = input.num_rows();
let columns = input.columns().copied().collect::<Vec<_>>();
// 2. select
Expand All @@ -151,12 +161,8 @@ impl ProverEvaluate for SliceExec {
.expect("Failed to create table from iterator");
let mut one_eval_lengths = input_one_eval_lengths;
one_eval_lengths.extend(vec![output_length, offset_index, max_index]);
(res, one_eval_lengths)
}

fn first_round_evaluate(&self, builder: &mut FirstRoundBuilder) {
self.input.first_round_evaluate(builder);
builder.request_post_result_challenges(2);
(res, one_eval_lengths)
}

#[tracing::instrument(name = "SliceExec::prover_evaluate", level = "debug", skip_all)]
Expand Down
154 changes: 132 additions & 22 deletions crates/proof-of-sql/src/sql/proof_plans/slice_exec_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ use crate::{
base::{
database::{
owned_table_utility::*, table_utility::*, ColumnField, ColumnType, OwnedTable,
OwnedTableTestAccessor, TableTestAccessor, TestAccessor,
OwnedTableTestAccessor, TableRef, TableTestAccessor, TestAccessor,
},
map::{indexmap, IndexMap},
math::decimal::Precision,
proof::ProofError,
scalar::Curve25519Scalar,
},
sql::{
proof::{
exercise_verification, ProvableQueryResult, ProverEvaluate, VerifiableQueryResult,
exercise_verification, FirstRoundBuilder, ProvableQueryResult, ProverEvaluate,
QueryError, VerifiableQueryResult,
},
proof_exprs::{test_utility::*, DynProofExpr},
},
Expand Down Expand Up @@ -65,7 +67,7 @@ fn we_can_prove_and_get_the_correct_empty_result_from_a_slice_exec() {
}

#[test]
fn we_can_get_an_empty_result_from_a_slice_on_an_empty_table_using_result_evaluate() {
fn we_can_get_an_empty_result_from_a_slice_on_an_empty_table_using_first_round_evaluate() {
let alloc = Bump::new();
let data = table([
borrowed_bigint("a", [0; 0], &alloc),
Expand Down Expand Up @@ -100,10 +102,13 @@ fn we_can_get_an_empty_result_from_a_slice_on_an_empty_table_using_result_evalua
ColumnType::Decimal75(Precision::new(75).unwrap(), 0),
),
];
let res: OwnedTable<Curve25519Scalar> =
ProvableQueryResult::from(expr.result_evaluate(&alloc, &table_map).0)
.to_owned_table(fields)
.unwrap();
let first_round_builder = &mut FirstRoundBuilder::new();
let res: OwnedTable<Curve25519Scalar> = ProvableQueryResult::from(
expr.first_round_evaluate(first_round_builder, &alloc, &table_map)
.0,
)
.to_owned_table(fields)
.unwrap();
let expected: OwnedTable<Curve25519Scalar> = owned_table([
bigint("b", [0; 0]),
int128("c", [0; 0]),
Expand All @@ -115,7 +120,7 @@ fn we_can_get_an_empty_result_from_a_slice_on_an_empty_table_using_result_evalua
}

#[test]
fn we_can_get_an_empty_result_from_a_slice_using_result_evaluate() {
fn we_can_get_an_empty_result_from_a_slice_using_first_round_evaluate() {
let alloc = Bump::new();
let data = table([
borrowed_bigint("a", [1, 4, 5, 2, 5], &alloc),
Expand Down Expand Up @@ -150,10 +155,13 @@ fn we_can_get_an_empty_result_from_a_slice_using_result_evaluate() {
ColumnType::Decimal75(Precision::new(1).unwrap(), 0),
),
];
let res: OwnedTable<Curve25519Scalar> =
ProvableQueryResult::from(expr.result_evaluate(&alloc, &table_map).0)
.to_owned_table(fields)
.unwrap();
let first_round_builder = &mut FirstRoundBuilder::new();
let res: OwnedTable<Curve25519Scalar> = ProvableQueryResult::from(
expr.first_round_evaluate(first_round_builder, &alloc, &table_map)
.0,
)
.to_owned_table(fields)
.unwrap();
let expected: OwnedTable<Curve25519Scalar> = owned_table([
bigint("b", [0; 0]),
int128("c", [0; 0]),
Expand All @@ -165,7 +173,7 @@ fn we_can_get_an_empty_result_from_a_slice_using_result_evaluate() {
}

#[test]
fn we_can_get_no_columns_from_a_slice_with_empty_input_using_result_evaluate() {
fn we_can_get_no_columns_from_a_slice_with_empty_input_using_first_round_evaluate() {
let alloc = Bump::new();
let data = table([
borrowed_bigint("a", [1, 4, 5, 2, 5], &alloc),
Expand All @@ -187,16 +195,19 @@ fn we_can_get_no_columns_from_a_slice_with_empty_input_using_result_evaluate() {
None,
);
let fields = &[];
let res: OwnedTable<Curve25519Scalar> =
ProvableQueryResult::from(expr.result_evaluate(&alloc, &table_map).0)
.to_owned_table(fields)
.unwrap();
let first_round_builder = &mut FirstRoundBuilder::new();
let res: OwnedTable<Curve25519Scalar> = ProvableQueryResult::from(
expr.first_round_evaluate(first_round_builder, &alloc, &table_map)
.0,
)
.to_owned_table(fields)
.unwrap();
let expected = OwnedTable::try_new(IndexMap::default()).unwrap();
assert_eq!(res, expected);
}

#[test]
fn we_can_get_the_correct_result_from_a_slice_using_result_evaluate() {
fn we_can_get_the_correct_result_from_a_slice_using_first_round_evaluate() {
let alloc = Bump::new();
let data = table([
borrowed_bigint("a", [1, 4, 5, 2, 5], &alloc),
Expand Down Expand Up @@ -230,10 +241,13 @@ fn we_can_get_the_correct_result_from_a_slice_using_result_evaluate() {
ColumnType::Decimal75(Precision::new(1).unwrap(), 0),
),
];
let res: OwnedTable<Curve25519Scalar> =
ProvableQueryResult::from(expr.result_evaluate(&alloc, &table_map).0)
.to_owned_table(fields)
.unwrap();
let first_round_builder = &mut FirstRoundBuilder::new();
let res: OwnedTable<Curve25519Scalar> = ProvableQueryResult::from(
expr.first_round_evaluate(first_round_builder, &alloc, &table_map)
.0,
)
.to_owned_table(fields)
.unwrap();
let expected: OwnedTable<Curve25519Scalar> = owned_table([
bigint("b", [5]),
int128("c", [5]),
Expand Down Expand Up @@ -434,3 +448,99 @@ fn we_can_prove_another_nested_slice_exec_with_no_rows() {
]);
assert_eq!(res, expected);
}

#[test]
fn we_can_create_and_prove_a_slice_exec_on_top_of_a_table_exec() {
let alloc = Bump::new();
let table_ref = TableRef::new("namespace.table_name".parse().unwrap());
let plan = slice_exec(
table_exec(
table_ref,
vec![
ColumnField::new("language_rank".parse().unwrap(), ColumnType::BigInt),
ColumnField::new("language_name".parse().unwrap(), ColumnType::VarChar),
ColumnField::new("space_and_time".parse().unwrap(), ColumnType::VarChar),
],
),
1,
Some(4),
);
let accessor = TableTestAccessor::<InnerProductProof>::new_from_table(
table_ref,
table([
borrowed_bigint("language_rank", [0_i64, 1, 2, 3], &alloc),
borrowed_varchar(
"language_name",
["English", "Español", "Português", "Français"],
&alloc,
),
borrowed_varchar(
"space_and_time",
[
"space and time",
"espacio y tiempo",
"espaço e tempo",
"espace et temps",
],
&alloc,
),
]),
0_usize,
(),
);
let verifiable_res = VerifiableQueryResult::new(&plan, &accessor, &());
exercise_verification(&verifiable_res, &plan, &accessor, table_ref);
let res = verifiable_res.verify(&plan, &accessor, &()).unwrap().table;
let expected = owned_table([
bigint("language_rank", [1_i64, 2, 3]),
varchar("language_name", ["Español", "Português", "Français"]),
varchar(
"space_and_time",
["espacio y tiempo", "espaço e tempo", "espace et temps"],
),
]);
assert_eq!(res, expected);
}

#[test]
fn we_can_create_and_prove_a_slice_exec_on_top_of_an_empty_exec() {
let empty_table = owned_table([]);
let t = "sxt.t".parse().unwrap();
let accessor = OwnedTableTestAccessor::<InnerProductProof>::new_empty_with_setup(());
let expr = slice_exec(empty_exec(), 3, Some(2));
let res = VerifiableQueryResult::new(&expr, &accessor, &());
exercise_verification(&res, &expr, &accessor, t);
let res = res.verify(&expr, &accessor, &()).unwrap().table;
assert_eq!(res, empty_table);
}

#[test]
fn we_cannot_prove_a_slice_exec_if_it_has_groupby_as_input_for_now() {
let data = owned_table([
bigint("a", [1, 2, 2, 1, 2]),
bigint("b", [99, 99, 99, 99, 0]),
bigint("c", [101, 102, 103, 104, 105]),
]);
let t = "sxt.t".parse().unwrap();
let mut accessor = OwnedTableTestAccessor::<InnerProductProof>::new_empty_with_setup(());
accessor.add_table(t, data, 0);
let expr = slice_exec(
group_by(
cols_expr(t, &["a"], &accessor),
vec![sum_expr(column(t, "c", &accessor), "sum_c")],
"__count__",
tab(t),
equal(column(t, "b", &accessor), const_int128(99)),
),
2,
None,
);
let res: VerifiableQueryResult<InnerProductProof> =
VerifiableQueryResult::new(&expr, &accessor, &());
assert!(matches!(
res.verify(&expr, &accessor, &()),
Err(QueryError::ProofError {
source: ProofError::UnsupportedError { .. }
})
));
}
8 changes: 7 additions & 1 deletion crates/proof-of-sql/src/sql/proof_plans/test_utility.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
use super::{DynProofPlan, FilterExec, GroupByExec, ProjectionExec, SliceExec, TableExec};
use super::{
DynProofPlan, EmptyExec, FilterExec, GroupByExec, ProjectionExec, SliceExec, TableExec,
};
use crate::{
base::database::{ColumnField, TableRef},
sql::proof_exprs::{AliasedDynProofExpr, ColumnExpr, DynProofExpr, TableExpr},
};

pub fn empty_exec() -> DynProofPlan {
DynProofPlan::Empty(EmptyExec::new())
}

pub fn table_exec(table_ref: TableRef, schema: Vec<ColumnField>) -> DynProofPlan {
DynProofPlan::Table(TableExec::new(table_ref, schema))
}
Expand Down

0 comments on commit e4bb220

Please sign in to comment.