fix: address review comments

crates/proof-of-sql/src/base/database/table.rs

@@ -1,5 +1,10 @@
-use super::Column;
-use crate::base::{map::IndexMap, scalar::Scalar};
+use super::{Column, ColumnRef, DataAccessor, TableRef};
+use crate::base::{
+    map::{IndexMap, IndexSet},
+    scalar::Scalar,
+};
+use alloc::vec;
+use bumpalo::Bump;
 use proof_of_sql_parser::Identifier;
 use snafu::Snafu;
 
@@ -24,7 +29,8 @@ impl<'a, S: Scalar> Table<'a, S> {
     /// Creates a new [`Table`].
     pub fn try_new(table: IndexMap<Identifier, Column<'a, S>>) -> Result<Self, TableError> {
         if table.is_empty() {
-            return Ok(Self { table, num_rows: 0 });
+            // `EmptyExec` should have one row for queries such as `SELECT 1`.
+            return Ok(Self { table, num_rows: 1 });
         }
         let num_rows = table[0].len();
         if table.values().any(|column| column.len() != num_rows) {
@@ -39,6 +45,34 @@ impl<'a, S: Scalar> Table<'a, S> {
     ) -> Result<Self, TableError> {
         Self::try_new(IndexMap::from_iter(iter))
     }
+    /// Creates a new [`Table`] from a [`DataAccessor`], [`TableRef`] and [`ColumnRef`]s.
+    ///
+    /// Columns are retrieved from the [`DataAccessor`] using the provided [`ColumnRef`]s.
+    /// # Panics
+    /// Missing columns or column length mismatches can occur if the accessor doesn't
+    /// contain the necessary columns. In practice, this should not happen.
+    pub(crate) fn from_columns(
+        column_refs: &IndexSet<ColumnRef>,
+        table_ref: TableRef,
+        accessor: &'a dyn DataAccessor<S>,
+        alloc: &'a Bump,
+    ) -> Self {
+        if column_refs.is_empty() {
+            // TODO: Currently we have to have non-empty column references to have a non-empty table
+            // to evaluate `ProofExpr`s on. Once we restrict [`DataAccessor`] to [`TableExec`]
+            // and use input `DynProofPlan`s we should no longer need this.
+            let input_length = accessor.get_length(table_ref);
+            let bogus_vec = vec![true; input_length];
+            let bogus_col = Column::Boolean(alloc.alloc_slice_copy(&bogus_vec));
+            Table::<'a, S>::try_from_iter(core::iter::once(("bogus".parse().unwrap(), bogus_col)))
+        } else {
+            Table::<'a, S>::try_from_iter(column_refs.into_iter().map(|column_ref| {
+                let column = accessor.get_column(*column_ref);
+                (column_ref.column_id(), column)
+            }))
+        }
+        .expect("Failed to create table from column references")
+    }
     /// Number of columns in the table.
     #[must_use]
     pub fn num_columns(&self) -> usize {

crates/proof-of-sql/src/base/database/table_test.rs

@@ -13,7 +13,7 @@ use proof_of_sql_parser::{
 fn we_can_create_a_table_with_no_columns() {
     let table = Table::<TestScalar>::try_new(IndexMap::default()).unwrap();
     assert_eq!(table.num_columns(), 0);
-    assert_eq!(table.num_rows(), None);
+    assert_eq!(table.num_rows(), 1);
 }
 #[test]
 fn we_can_create_an_empty_table() {

crates/proof-of-sql/src/base/database/table_test_accessor.rs

@@ -111,11 +111,7 @@ impl<CP: CommitmentEvaluationProof> MetadataAccessor for TableTestAccessor<'_, C
     ///
     /// Will panic if the `table_ref` is not found in `self.tables`, indicating that an invalid reference was provided.
     fn get_length(&self, table_ref: TableRef) -> usize {
-        self.tables
-            .get(&table_ref)
-            .unwrap()
-            .0
-            .num_rows()
+        self.tables.get(&table_ref).unwrap().0.num_rows()
     }
     ///
     /// # Panics

crates/proof-of-sql/src/sql/proof_exprs/and_expr.rs

@@ -50,9 +50,7 @@ impl ProofExpr for AndExpr {
         let rhs_column: Column<'a, S> = self.rhs.result_evaluate(alloc, table);
         let lhs = lhs_column.as_boolean().expect("lhs is not boolean");
         let rhs = rhs_column.as_boolean().expect("rhs is not boolean");
-        Column::Boolean(
-            alloc.alloc_slice_fill_with(table.num_rows(), |i| lhs[i] && rhs[i]),
-        )
+        Column::Boolean(alloc.alloc_slice_fill_with(table.num_rows(), |i| lhs[i] && rhs[i]))
     }
 
     #[tracing::instrument(name = "AndExpr::prover_evaluate", level = "debug", skip_all)]

crates/proof-of-sql/src/sql/proof_exprs/equals_expr.rs

@@ -51,11 +51,7 @@ impl ProofExpr for EqualsExpr {
         let rhs_scale = self.rhs.data_type().scale().unwrap_or(0);
         let res = scale_and_subtract(alloc, lhs_column, rhs_column, lhs_scale, rhs_scale, true)
             .expect("Failed to scale and subtract");
-        Column::Boolean(result_evaluate_equals_zero(
-            table.num_rows(),
-            alloc,
-            res,
-        ))
+        Column::Boolean(result_evaluate_equals_zero(table.num_rows(), alloc, res))
     }
 
     #[tracing::instrument(name = "EqualsExpr::prover_evaluate", level = "debug", skip_all)]

crates/proof-of-sql/src/sql/proof_exprs/or_expr.rs

@@ -48,12 +48,7 @@ impl ProofExpr for OrExpr {
         let rhs_column: Column<'a, S> = self.rhs.result_evaluate(alloc, table);
         let lhs = lhs_column.as_boolean().expect("lhs is not boolean");
         let rhs = rhs_column.as_boolean().expect("rhs is not boolean");
-        Column::Boolean(result_evaluate_or(
-            table.num_rows(),
-            alloc,
-            lhs,
-            rhs,
-        ))
+        Column::Boolean(result_evaluate_or(table.num_rows(), alloc, lhs, rhs))
     }
 
     #[tracing::instrument(name = "OrExpr::prover_evaluate", level = "debug", skip_all)]

crates/proof-of-sql/src/sql/proof_plans/filter_exec.rs

@@ -143,11 +143,8 @@ impl ProverEvaluate for FilterExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-            let column = accessor.get_column(*column_ref);
-            (column_ref.column_id(), column)
-        }))
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. selection
         let selection_column: Column<'a, S> = self.where_clause.result_evaluate(alloc, &used_table);
         let selection = selection_column
@@ -179,11 +176,8 @@ impl ProverEvaluate for FilterExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-            let column = accessor.get_column(*column_ref);
-            (column_ref.column_id(), column)
-        }))
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. selection
         let selection_column: Column<'a, S> =
             self.where_clause

crates/proof-of-sql/src/sql/proof_plans/filter_exec_test_dishonest_prover.rs

@@ -39,11 +39,8 @@ impl ProverEvaluate for DishonestFilterExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-            let column = accessor.get_column(*column_ref);
-            (column_ref.column_id(), column)
-        }))
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. selection
         let selection_column: Column<'a, S> = self.where_clause.result_evaluate(alloc, &used_table);
         let selection = selection_column
@@ -78,11 +75,8 @@ impl ProverEvaluate for DishonestFilterExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-            let column = accessor.get_column(*column_ref);
-            (column_ref.column_id(), column)
-        }))
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. selection
         let selection_column: Column<'a, S> =
             self.where_clause

crates/proof-of-sql/src/sql/proof_plans/group_by_exec.rs

@@ -202,21 +202,8 @@ impl ProverEvaluate for GroupByExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = if column_refs.is_empty() {
-            //TODO: Currently we have to have non-empty column references to have a non-empty table
-            // to evaluate `ProofExpr`s on. Once we restrict [`DataAccessor`] to [`TableExec`]
-            // and use input `DynProofPlan`s we should no longer need this.
-            let input_length = accessor.get_length(self.table.table_ref);
-            let bogus_vec = vec![true; input_length];
-            let bogus_col = Column::Boolean(alloc.alloc_slice_copy(&bogus_vec));
-            Table::<'a, S>::try_from_iter(core::iter::once(("bogus".parse().unwrap(), bogus_col)))
-        } else {
-            Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-                let column = accessor.get_column(*column_ref);
-                (column_ref.column_id(), column)
-            }))
-        }
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. selection
         let selection_column: Column<'a, S> = self.where_clause.result_evaluate(alloc, &used_table);
 
@@ -264,21 +251,8 @@ impl ProverEvaluate for GroupByExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = if column_refs.is_empty() {
-            //TODO: Currently we have to have non-empty column references to have a non-empty table
-            // to evaluate `ProofExpr`s on. Once we restrict [`DataAccessor`] to [`TableExec`]
-            // and use input `DynProofPlan`s we should no longer need this.
-            let input_length = accessor.get_length(self.table.table_ref);
-            let bogus_vec = vec![true; input_length];
-            let bogus_col = Column::Boolean(alloc.alloc_slice_copy(&bogus_vec));
-            Table::<'a, S>::try_from_iter(core::iter::once(("bogus".parse().unwrap(), bogus_col)))
-        } else {
-            Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-                let column = accessor.get_column(*column_ref);
-                (column_ref.column_id(), column)
-            }))
-        }
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. selection
         let selection_column: Column<'a, S> =
             self.where_clause

crates/proof-of-sql/src/sql/proof_plans/projection_exec.rs

@@ -91,11 +91,8 @@ impl ProverEvaluate for ProjectionExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-            let column = accessor.get_column(*column_ref);
-            (column_ref.column_id(), column)
-        }))
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         let columns: Vec<_> = self
             .aliased_results
             .iter()
@@ -119,11 +116,8 @@ impl ProverEvaluate for ProjectionExec {
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {
         let column_refs = self.get_column_references();
-        let used_table = Table::<'a, S>::try_from_iter(column_refs.iter().map(|column_ref| {
-            let column = accessor.get_column(*column_ref);
-            (column_ref.column_id(), column)
-        }))
-        .expect("Failed to create table from column references");
+        let used_table =
+            Table::<'a, S>::from_columns(&column_refs, self.table.table_ref, accessor, alloc);
         // 1. Evaluate result expressions
         let res: Vec<_> = self
             .aliased_results

crates/proof-of-sql/src/sql/proof_plans/table_exec.rs

@@ -85,7 +85,6 @@ impl ProverEvaluate for TableExec {
     #[tracing::instrument(name = "TableExec::result_evaluate", level = "debug", skip_all)]
     fn result_evaluate<'a, S: Scalar>(
         &self,
-        _input_length: usize,
         _alloc: &'a Bump,
         accessor: &'a dyn DataAccessor<S>,
     ) -> Vec<Column<'a, S>> {