Skip to content

Commit 9ff6da8

Browse files
committed
Fix filter operator and update parallel aggregation test
- VectorizedFilterOperator: fix row_count not being set after appending rows, causing FilterAllMatch and PipelinedBatches tests to fail - BinaryExpr::evaluate_vectorized: add Ge (>=) operator support for INT64 columns with constant, fixing val >= 0 filter conditions - VectorizedGroupByTests.ParallelAggregationCorrectness: use std::map for order-independent verification since TEXT key output order is non-deterministic with hash aggregation
1 parent 79934b0 commit 9ff6da8

3 files changed

Lines changed: 44 additions & 15 deletions

File tree

include/executor/vectorized_operator.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ class VectorizedFilterOperator : public VectorizedOperator {
213213
dest_col.append(src_col.get(r));
214214
}
215215
}
216+
// Update row count after appending
217+
out_batch.set_row_count(out_batch.row_count() + selection.size());
216218
}
217219

218220
if (out_batch.row_count() > 0) {

src/parser/expression.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,20 @@ void BinaryExpr::evaluate_vectorized(const executor::VectorBatch& batch,
118118
}
119119
return;
120120
}
121+
if (op_ == TokenType::Ge) {
122+
auto& bool_res = dynamic_cast<executor::NumericVector<bool>&>(result);
123+
bool_res.resize(row_count);
124+
uint8_t* res_data = bool_res.raw_data_mut();
125+
for (size_t i = 0; i < row_count; ++i) {
126+
if (num_src.is_null(i)) {
127+
bool_res.set_null(i, true);
128+
} else {
129+
res_data[i] = static_cast<uint8_t>(src_data[i] >= const_val);
130+
bool_res.set_null(i, false);
131+
}
132+
}
133+
return;
134+
}
121135
}
122136
}
123137
}

tests/vectorized_operator_tests.cpp

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,11 +1543,11 @@ TEST_F(VectorizedGroupByTests, VectorizedHashJoinFull) {
15431543
}
15441544

15451545
TEST_F(VectorizedGroupByTests, ParallelAggregationCorrectness) {
1546-
// Test that parallel aggregation (num_threads > 1) produces correct results
1547-
// This test creates a ThreadPool with 4 threads and verifies GROUP BY
1548-
// produces the same results as expected (computed manually)
1546+
// Test that parallel aggregation (num_threads > 1) produces correct results.
1547+
// We verify correctness by checking that the total counts and sums across all
1548+
// groups match expected values, without relying on specific output ordering.
15491549

1550-
// Use TEXT column to ensure hash aggregation path (not DirectIndexAgg)
1550+
// Use TEXT column to ensure OpenAddressHashAgg path (tests parallel hash aggregation)
15511551
Schema schema;
15521552
schema.add_column("cat", common::ValueType::TYPE_TEXT);
15531553
schema.add_column("val", common::ValueType::TYPE_INT64);
@@ -1586,19 +1586,32 @@ TEST_F(VectorizedGroupByTests, ParallelAggregationCorrectness) {
15861586
std::move(out_schema), thread_pool);
15871587

15881588
auto result = VectorBatch::create(groupby.output_schema());
1589-
ASSERT_TRUE(groupby.next_batch(*result));
1590-
ASSERT_EQ(result->row_count(), 10); // 10 distinct groups
15911589

1592-
// Verify results: each category should have count=10 and sum = 10*(catIdx+1) + 45 = 10*catIdx +
1593-
// 55 Actually for cat0 (i=0,10,20,...90): sum = 1+11+21+...+91 = 460 For cat1
1594-
// (i=1,11,21,...91): sum = 2+12+22+...+92 = 470, etc.
1595-
for (size_t i = 0; i < 10; ++i) {
1596-
int64_t cnt = result->get_column(1).get(i).as_int64();
1597-
int64_t sum = result->get_column(2).get(i).as_int64();
1590+
// Collect all results into maps keyed by category string
1591+
std::map<std::string, std::pair<int64_t, int64_t>> results; // cat -> (count, sum)
1592+
1593+
while (groupby.next_batch(*result)) {
1594+
for (size_t i = 0; i < result->row_count(); ++i) {
1595+
std::string cat = result->get_column(0).get(i).as_text();
1596+
int64_t cnt = result->get_column(1).get(i).as_int64();
1597+
int64_t sum = result->get_column(2).get(i).as_int64();
1598+
results[cat] = {cnt, sum};
1599+
}
1600+
result->clear();
1601+
}
15981602

1599-
EXPECT_EQ(cnt, 10) << "Count mismatch for category " << i;
1600-
// Sum formula: (i+1) + (i+11) + ... + (i+91) = 10*i + (1+11+21+...+91) = 10*i + 460
1601-
EXPECT_EQ(sum, 10 * static_cast<int64_t>(i) + 460) << "Sum mismatch for category " << i;
1603+
// Verify we got 10 groups
1604+
ASSERT_EQ(results.size(), 10) << "Should have 10 distinct groups";
1605+
1606+
// Verify each group has count=10 and correct sum
1607+
// cat0: values 1,11,21,...,91 sum to 460
1608+
// cat1: values 2,12,22,...,92 sum to 470, etc.
1609+
for (int i = 0; i < 10; ++i) {
1610+
std::string cat = "cat" + std::to_string(i);
1611+
ASSERT_TRUE(results.count(cat) > 0) << "Category " << cat << " missing";
1612+
EXPECT_EQ(results[cat].first, 10) << "Count mismatch for " << cat;
1613+
EXPECT_EQ(results[cat].second, static_cast<int64_t>(10 * i + 460))
1614+
<< "Sum mismatch for " << cat;
16021615
}
16031616
}
16041617

0 commit comments

Comments
 (0)