Skip to content

Commit

Permalink
fix: address reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
iajoiner committed Nov 12, 2024
1 parent 1d5903e commit 1eae264
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 22 deletions.
10 changes: 3 additions & 7 deletions crates/proof-of-sql/src/base/database/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,10 @@ impl<'a, S: Scalar> Table<'a, S> {
pub fn num_columns(&self) -> usize {
self.table.len()
}
/// Number of rows in the table.
/// Number of rows in the table. For an empty table, this will return `None`.
#[must_use]
pub fn num_rows(&self) -> usize {
if self.table.is_empty() {
0
} else {
self.table[0].len()
}
pub fn num_rows(&self) -> Option<usize> {
(!self.table.is_empty()).then(|| self.table[0].len())
}
/// Whether the table has no columns.
#[must_use]
Expand Down
24 changes: 11 additions & 13 deletions crates/proof-of-sql/src/base/database/table_test.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use crate::{
base::{
database::{table_utility::*, Column, Table, TableError},
map::IndexMap,
scalar::test_scalar::TestScalar,
},
proof_primitive::dory::DoryScalar,
use crate::base::{
database::{table_utility::*, Column, Table, TableError},
map::IndexMap,
scalar::test_scalar::TestScalar,
};
use bumpalo::Bump;
use proof_of_sql_parser::{
Expand All @@ -16,11 +13,12 @@ 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);
}
#[test]
fn we_can_create_an_empty_table() {
let alloc = Bump::new();
let borrowed_table = table::<DoryScalar>([
let borrowed_table = table::<TestScalar>([
borrowed_bigint("bigint", [0; 0], &alloc),
borrowed_int128("decimal", [0; 0], &alloc),
borrowed_varchar("varchar", ["0"; 0], &alloc),
Expand All @@ -46,7 +44,7 @@ fn we_can_create_an_empty_table() {
fn we_can_create_a_table_with_data() {
let alloc = Bump::new();

let borrowed_table = table::<DoryScalar>([
let borrowed_table = table::<TestScalar>([
borrowed_bigint(
"bigint",
[0_i64, 1, 2, 3, 4, 5, 6, i64::MIN, i64::MAX],
Expand Down Expand Up @@ -102,14 +100,14 @@ fn we_can_create_a_table_with_data() {
.map(|&s| alloc.alloc_str(s) as &str)
.collect();
let varchar_str_slice = alloc.alloc_slice_clone(&varchar_data);
let varchar_scalars: Vec<DoryScalar> = varchar_data.iter().map(Into::into).collect();
let varchar_scalars: Vec<TestScalar> = varchar_data.iter().map(Into::into).collect();
let varchar_scalars_slice = alloc.alloc_slice_clone(&varchar_scalars);
expected_table.insert(
Identifier::try_new("varchar").unwrap(),
Column::VarChar((varchar_str_slice, varchar_scalars_slice)),
);

let scalar_data: Vec<DoryScalar> = (0..=8).map(DoryScalar::from).collect();
let scalar_data: Vec<TestScalar> = (0..=8).map(TestScalar::from).collect();
let scalar_slice = alloc.alloc_slice_copy(&scalar_data);
expected_table.insert(
Identifier::try_new("scalar").unwrap(),
Expand Down Expand Up @@ -165,7 +163,7 @@ fn we_get_inequality_between_tables_with_differing_column_order() {
fn we_get_inequality_between_tables_with_differing_data() {
let alloc = Bump::new();

let table_a: Table<'_, DoryScalar> = table([
let table_a: Table<'_, TestScalar> = table([
borrowed_bigint("a", [0], &alloc),
borrowed_int128("b", [0], &alloc),
borrowed_varchar("c", ["0"], &alloc),
Expand All @@ -179,7 +177,7 @@ fn we_get_inequality_between_tables_with_differing_data() {
),
]);

let table_b: Table<'_, DoryScalar> = table([
let table_b: Table<'_, TestScalar> = table([
borrowed_bigint("a", [1], &alloc),
borrowed_int128("b", [0], &alloc),
borrowed_varchar("c", ["0"], &alloc),
Expand Down
7 changes: 6 additions & 1 deletion crates/proof-of-sql/src/base/database/table_test_accessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ 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()
.unwrap_or(0)
}
///
/// # Panics
Expand Down
1 change: 0 additions & 1 deletion crates/proof-of-sql/src/base/database/table_utility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ pub fn borrowed_int<S: Scalar>(
///
/// # Panics
/// - Panics if `name.parse()` fails to convert the name into an `Identifier`.
#[allow(clippy::missing_panics_doc)]
pub fn borrowed_bigint<S: Scalar>(
name: impl Deref<Target = str>,
data: impl IntoIterator<Item = impl Into<i64>>,
Expand Down

0 comments on commit 1eae264

Please sign in to comment.