Optimize min/max implementation using native PartialOrd by Copilot · Pull Request #9 · streamingfast/substreams-sink-database-changes

Copilot · 2026-01-13T14:43:27Z

BLO-806: Optimize min/max Implementation ✅

Successfully implemented the NumericComparable trait to optimize min/max operations by leveraging BigDecimal's native PartialOrd implementations.

Implementation Checklist

Key Changes

1. New Trait: NumericComparable

Extends ToBigDecimal trait (consistent with NumericAddable pattern)
Enables direct comparison with BigDecimal without string conversion
Leverages BigDecimal's native PartialOrd implementations for zero-allocation comparisons

2. Optimized Methods

max() and min() signatures now match add()/sub() pattern: <T: NumericComparable>
Direct comparison via cmp_to_big_decimal() for integers
Uses to_big_decimal().to_string() only when storing values
Breaking change: String types no longer accepted - users must convert strings to numeric types explicitly

3. Comprehensive Testing

All 81 tests pass
All tests use numeric types (integers, BigDecimal, BigInt)
String-based tests removed - API now requires explicit type conversion

Performance Impact

Before:

value.to_value()              // Allocation #1: i64 -> String
BigDecimal::from_str(&value)  // Allocation #2 + parsing
compare BigDecimals

After (for integers):

value.cmp_to_big_decimal(&existing)  // Zero allocation (native PartialOrd)
value.to_big_decimal().to_string()   // Only if storing

Results:

✅ Zero allocations for integer comparisons (was 2 allocations + parsing)
✅ Eliminates string parsing overhead entirely for integers
✅ API consistent with add()/sub() methods
✅ More type-safe - no implicit string conversions
✅ Better error messages with actual type names via stringify!

API Changes

Breaking Change: The max() and min() methods no longer accept string types directly. Users must convert strings to appropriate numeric types before calling these methods.

Before:

row.max("price", "100");  // No longer compiles

After:

row.max("price", 100i64);  // Use numeric types directly
// or convert explicitly:
row.max("price", BigDecimal::from_str("100.50").unwrap());

Verification

✅ All 81 tests pass
✅ CodeQL: 0 security vulnerabilities
✅ Comparison semantics verified
✅ API consistent with add/sub pattern

Files Modified

src/numeric/traits.rs: NumericComparable extends ToBigDecimal
src/numeric/impls.rs: Refined implementations, added missing types
src/numeric/mod.rs: Export NumericComparable
src/tables.rs: Updated signatures and all tests

Fixes: https://linear.app/streamingfast/issue/BLO-806/optimize-minmax-implementation

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Added NumericComparable trait to enable zero-allocation comparisons - Implemented trait for all integer types using BigDecimal's native PartialOrd - Updated max() and min() methods to use NumericComparable - All existing tests pass Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>

- Added tests for i32, i64, u32 max/min operations - Added tests for BigDecimal and BigInt max/min - Added tests for string max/min to ensure backwards compatibility - Added tests for mixed integer types - All 82 tests pass successfully Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>

- Added explicit test to verify integer comparison ordering - Confirms that 50 < 100, 200 > 100, and 100 == 100 work correctly - All 83 tests pass successfully Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>

maoueh · 2026-01-13T15:52:35Z

+// i128 and u128 - use BigDecimal's native PartialOrd for these types
+impl NumericComparable for i128 {
+    fn cmp_to_big_decimal(&self, other: &BigDecimal) -> Ordering {
+        self.partial_cmp(other)
+            .expect("BigDecimal comparison should always succeed for i128")
+    }
+}
+
+impl NumericComparable for u128 {
+    fn cmp_to_big_decimal(&self, other: &BigDecimal) -> Ordering {
+        self.partial_cmp(other)
+            .expect("BigDecimal comparison should always succeed for u128")
+    }
+}


Seems to me those can be integrated directly into impl_numeric_comparable_for_integer! macro call, implementation is that same.

Done in commit a1724ba. Integrated i128 and u128 into the macro call since they use the same implementation pattern with native PartialOrd.

maoueh · 2026-01-13T15:53:09Z

+                    // Direct comparison using BigDecimal's PartialOrd<$t>
+                    // This is zero-allocation for integers
+                    self.partial_cmp(other)
+                        .expect("BigDecimal comparison should always succeed for integers")


Replace for integers with for <type> (with correct Rust macro stringify syntax)

Done in commit a1724ba. Updated to use stringify!($t) in the panic message for accurate type names.

maoueh · 2026-01-13T15:55:11Z

+// String implementations - must parse to BigDecimal
+impl NumericComparable for String {
+    fn cmp_to_big_decimal(&self, other: &BigDecimal) -> Ordering {
+        let self_bd = BigDecimal::from_str(self).unwrap_or_else(|_| {
+            panic!("min/max() requires a valid numeric value, got: {}", self)
+        });
+        self_bd.cmp(other)
+    }
+}
+
+impl NumericComparable for &str {
+    fn cmp_to_big_decimal(&self, other: &BigDecimal) -> Ordering {
+        let self_bd = BigDecimal::from_str(self).unwrap_or_else(|_| {
+            panic!("min/max() requires a valid numeric value, got: {}", self)
+        });
+        self_bd.cmp(other)
+    }
+}


Since those panics, I think we shouldn't offer them, user/caller should perform the conversation hisself and use his required types.

Done in commit a1724ba. Removed String and &str implementations. Users must now convert strings to numeric types themselves before calling min/max.

maoueh · 2026-01-13T15:56:09Z

+///
+/// This trait leverages BigDecimal's native PartialOrd implementations for primitive
+/// types, enabling zero-allocation comparisons for integers.
+pub trait NumericComparable {


Make : ToBigDecimal like NumericAddable.

Done in commit a1724ba. Made NumericComparable extend ToBigDecimal trait, consistent with NumericAddable pattern.

maoueh · 2026-01-13T15:56:22Z

    /// Used with upsert_row() for tracking high values.
    /// Can only follow set() or another max() call on the same field.
-    pub fn max<T: ToDatabaseValue>(&mut self, name: &str, value: T) -> &mut Self {
+    pub fn max<T: NumericComparable + ToDatabaseValue>(&mut self, name: &str, value: T) -> &mut Self {


We will not accept string, use signature more similar to add/sub, adjust the tests as a consequence of the constraining of accepted types that are now reduced.

Done in commit a1724ba. Changed signature to <T: NumericComparable> (removed ToDatabaseValue). Updated all tests to use numeric types instead of strings.

maoueh · 2026-01-13T15:57:30Z

    /// Used with upsert_row() for tracking low values.
    /// Can only follow set() or another min() call on the same field.
-    pub fn min<T: ToDatabaseValue>(&mut self, name: &str, value: T) -> &mut Self {
+    pub fn min<T: NumericComparable + ToDatabaseValue>(&mut self, name: &str, value: T) -> &mut Self {


Same comment as max here, adjust accordingly.

Done in commit a1724ba. Changed min() signature to match max() - now <T: NumericComparable> only. Updated all related tests.

- Made NumericComparable extend ToBigDecimal trait - Integrated i128/u128 into macro (use native PartialOrd) - Added ToBigDecimal/NumericAddable for isize/usize/i128/u128 - Improved panic message to use stringify!($t) for type name - Removed String/&str NumericComparable implementations - Changed max/min signatures to match add/sub pattern (removed ToDatabaseValue) - Updated all tests to use integers instead of strings - Removed string-specific tests Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>

Initial plan

02626ad

Copilot AI assigned Copilot and maoueh Jan 13, 2026

Copilot started work on behalf of maoueh January 13, 2026 14:54 View session

Copilot AI and others added 3 commits January 13, 2026 15:00

Add verification test for comparison semantics

91d767b

- Added explicit test to verify integer comparison ordering - Confirms that 50 < 100, 200 > 100, and 100 == 100 work correctly - All 83 tests pass successfully Co-authored-by: maoueh <123014+maoueh@users.noreply.github.com>

Copilot AI requested a review from maoueh January 13, 2026 15:14

Copilot finished work on behalf of maoueh January 13, 2026 15:14

maoueh requested changes Jan 13, 2026

View reviewed changes

Copilot started work on behalf of maoueh January 13, 2026 15:58 View session

Copilot AI changed the title ~~[WIP] Optimize min/max implementation using native PartialOrd~~ Optimize min/max implementation using native PartialOrd Jan 13, 2026

Copilot finished work on behalf of maoueh January 13, 2026 16:06

Copilot AI requested a review from maoueh January 13, 2026 16:06

maoueh approved these changes Jan 13, 2026

View reviewed changes

maoueh marked this pull request as ready for review January 13, 2026 16:34

maoueh merged commit 03fd232 into develop Jan 13, 2026
3 checks passed

maoueh deleted the copilot/optimize-min-max-operations branch January 14, 2026 15:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize min/max implementation using native PartialOrd#9

Optimize min/max implementation using native PartialOrd#9
maoueh merged 5 commits intodevelopfrom
copilot/optimize-min-max-operations

Copilot AI commented Jan 13, 2026 •

edited

Loading

Uh oh!

maoueh Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

maoueh Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

maoueh Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

maoueh Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

maoueh Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

maoueh Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Copilot AI commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

BLO-806: Optimize min/max Implementation ✅

Implementation Checklist

Key Changes

Performance Impact

API Changes

Verification

Files Modified

Uh oh!

maoueh Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

maoueh Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

maoueh Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

maoueh Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

maoueh Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

maoueh Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Jan 13, 2026 •

edited

Loading