Skip to content

Commit

Permalink
fix: incorporate leftover feedback from previous PR (#17)
Browse files Browse the repository at this point in the history
# Rationale for this change

Incorporates a small amount of feedback from #12 

# What changes are included in this PR?

Corrects naming in tests and removes unsupported feature in
group_by_util

# Are these changes tested?

yes
  • Loading branch information
Dustin-Ray authored Jun 19, 2024
1 parent 623df7d commit 6c3aa58
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
16 changes: 8 additions & 8 deletions crates/proof-of-sql/src/base/commitment/committable_column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,24 +223,24 @@ mod tests {
#[test]
fn we_can_get_type_and_length_of_timestamp_column() {
// empty case
let smallint_committable_column =
let committable_column =
CommittableColumn::TimestampTZ(PoSQLTimeUnit::Second, PoSQLTimeZone::UTC, &[]);
assert_eq!(smallint_committable_column.len(), 0);
assert!(smallint_committable_column.is_empty());
assert_eq!(committable_column.len(), 0);
assert!(committable_column.is_empty());
assert_eq!(
smallint_committable_column.column_type(),
committable_column.column_type(),
ColumnType::TimestampTZ(PoSQLTimeUnit::Second, PoSQLTimeZone::UTC)
);

let smallint_committable_column = CommittableColumn::TimestampTZ(
let committable_column = CommittableColumn::TimestampTZ(
PoSQLTimeUnit::Second,
PoSQLTimeZone::UTC,
&[12, 34, 56],
);
assert_eq!(smallint_committable_column.len(), 3);
assert!(!smallint_committable_column.is_empty());
assert_eq!(committable_column.len(), 3);
assert!(!committable_column.is_empty());
assert_eq!(
smallint_committable_column.column_type(),
committable_column.column_type(),
ColumnType::TimestampTZ(PoSQLTimeUnit::Second, PoSQLTimeZone::UTC)
);
}
Expand Down
6 changes: 3 additions & 3 deletions crates/proof-of-sql/src/sql/ast/group_by_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ pub(super) fn sum_aggregate_column_by_index_counts<'a, S: Scalar>(
todo!("aggregation over decimals not yet supported")
}
Column::Scalar(col) => sum_aggregate_slice_by_index_counts(alloc, col, counts, indexes),
Column::VarChar(_) => unimplemented!("Cannot sum varchar columns"),
Column::TimestampTZ(_, _, col) => {
sum_aggregate_slice_by_index_counts(alloc, col, counts, indexes)
Column::VarChar(_) => panic!("Cannot sum varchar columns"),
Column::TimestampTZ(_, _, _) => {
panic!("Cannot sum varchar columns")
}
}
}
Expand Down

0 comments on commit 6c3aa58

Please sign in to comment.