Throw appropriate error if resampling dynamic schema with missing agg…

… column
man-group · Apr 19, 2024 · 6a4d98a · 6a4d98a
1 parent 74680dd
commit 6a4d98a
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 6 deletions.
diff --git a/cpp/arcticdb/processing/aggregation.cpp b/cpp/arcticdb/processing/aggregation.cpp
@@ -707,7 +707,9 @@ Column SortedAggregator<aggregation_operator, closed_boundary>::aggregate(const
                                 }
                             );
                         } else {
-                            // Column is missing from this row-slice due to dynamic schema
+                            // Column is missing from this row-slice due to dynamic schema, currently unsupported
+                            schema::raise<ErrorCode::E_UNSUPPORTED_COLUMN_TYPE>("Resample: Cannot aggregate column '{}' as it is missing from some row slices",
+                                                                                get_input_column_name().value);
                         }
                     }
                     // We were in the middle of aggregating a bucket when we ran out of index values
@@ -728,10 +730,9 @@ Column SortedAggregator<aggregation_operator, closed_boundary>::aggregate(const
                 }
             }
         );
-    } else {
-        // All input columns are nullopt - dynamic schema
     }
-    internal::check<ErrorCode::E_ASSERTION_FAILURE>(res.has_value(), "Dynamic schema not currently supported with resampling");
+    internal::check<ErrorCode::E_ASSERTION_FAILURE>(res.has_value(),
+                                                    "Should not be able to reach end of SortedAggregator without a column to return");
     return std::move(*res);
 }
 

diff --git a/cpp/arcticdb/processing/clause.cpp b/cpp/arcticdb/processing/clause.cpp
@@ -733,6 +733,8 @@ Composite<EntityIds> ResampleClause<closed_boundary>::process(Composite<EntityId
                                     input_agg_columns.emplace_back(column_with_strings);
                                 },
                                 [&input_agg_columns](const EmptyResult&) {
+                                    // Dynamic schema, missing column from this row-slice
+                                    // Not currently supported, but will be, hence the argument to aggregate being a vector of optionals
                                     input_agg_columns.emplace_back();
                                 },
                                 [](const auto&) {

diff --git a/python/tests/unit/arcticdb/version_store/test_resample.py b/python/tests/unit/arcticdb/version_store/test_resample.py
@@ -451,11 +451,10 @@ def test_resampling_dynamic_schema_types_changing(lmdb_version_store_dynamic_sch
     assert_frame_equal(expected, received)
 
 
-# TODO: Reduce amount of data
 def test_resampling_named_agg(lmdb_version_store_v1):
     lib = lmdb_version_store_v1
     sym = "test_resampling_named_agg"
-    index = pd.date_range("2023-01-01", "2023-01-31T23:59:59", freq="S")
+    index = pd.date_range("2023-01-01", "2023-01-03", freq="S")
     rng = np.random.default_rng()
     df = pd.DataFrame(
         {