Skip to content

Commit baed8c8

Browse files
test(ft_aggregate): Fix FirstValueReducerEdgeCasesTest test case
Signed-off-by: Riley Des <riley.desserre@improving.com>
1 parent 27d55cb commit baed8c8

1 file changed

Lines changed: 10 additions & 14 deletions

File tree

testing/ft_aggregate_exec_test.cc

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ TEST_F(AggregateExecTest, FirstValueReducerTest) {
357357
auto params = std::make_unique<AggregateParameters>(0);
358358
params->parse_vars_.index_interface_ = &fakeIndex;
359359
params->AddRecordAttribute("n1", "n1", indexes::IndexerType::kNumeric);
360-
params->AddRecordAttribute("n2", "n1", indexes::IndexerType::kNumeric);
360+
params->AddRecordAttribute("n2", "n2", indexes::IndexerType::kNumeric);
361361
auto parser = CreateAggregateParser();
362362
auto result = parser.Parse(*params, itr);
363363
EXPECT_FALSE(result.ok()) << tc.text_ << ": expected parse failure";
@@ -413,31 +413,27 @@ TEST_F(AggregateExecTest, FirstValueReducerEdgeCasesTest) {
413413
auto param =
414414
MakeStages("groupby 1 @n2 reduce first_value 4 @n1 BY @n2 ASC");
415415
RecordSet records(nullptr);
416-
// rec with n2=1 (best ASC) has nil n1 — that nil should be returned.
416+
// Same group key (n2=1.0) ties on the BY comparison, first-encountered
417+
// wins, so r1's nil n1 must be preserved rather than overwritten by r2.
417418
auto r1 = std::make_unique<Record>(2);
418419
r1->fields_[0] = expr::Value();
419420
r1->fields_[1] = expr::Value(1.0);
420421
records.emplace_back(std::move(r1));
421422
auto r2 = std::make_unique<Record>(2);
422423
r2->fields_[0] = expr::Value(100.0);
423-
r2->fields_[1] = expr::Value(5.0);
424+
r2->fields_[1] = expr::Value(1.0);
424425
records.emplace_back(std::move(r2));
425426
EXPECT_TRUE(param->stages_[0]->Execute(records).ok());
426-
ASSERT_EQ(records.size(), 2);
427+
ASSERT_EQ(records.size(), 1);
427428
std::cerr << "Results:\n";
428429
for (auto& rec : records) {
429430
std::cerr << *rec << "\n";
430431
}
431-
// Find the group with n2=1 and verify its result is nil.
432-
bool found = false;
433-
for (auto& rec : records) {
434-
if (rec->fields_.at(1).IsDouble() &&
435-
*rec->fields_.at(1).AsDouble() == 1.0) {
436-
EXPECT_TRUE(rec->fields_.at(2).IsNil());
437-
found = true;
438-
}
439-
}
440-
EXPECT_TRUE(found);
432+
// The single group (n2=1.0) should have its first_value result be nil.
433+
auto& rec = records.front();
434+
ASSERT_TRUE(rec->fields_.at(1).IsDouble());
435+
EXPECT_EQ(*rec->fields_.at(1).AsDouble(), 1.0);
436+
EXPECT_TRUE(rec->fields_.at(2).IsNil());
441437
}
442438

443439
// Tie-breaking: first encountered record wins (ASC).

0 commit comments

Comments
 (0)