feat: fix ConditionalRowFilter with FamilyNameRegexFilter#20060
feat: fix ConditionalRowFilter with FamilyNameRegexFilter#20060hardikbadjatiya wants to merge 8 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request adds a cleanup step in filterRow to remove empty columns and families, aligning the in-memory Bigtable test behavior with production. The reviewer identified a potential data corruption risk where in-place modification of fam.colNames could affect shared row copies, and noted incorrect indentation. A code suggestion was provided to allocate a new slice and fix the formatting.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Removed unnecessary comments and streamlined the explanation for cleaning up empty columns and families.
Remove empty column families after per-cell filtering to match production Bigtable behavior when evaluating ConditionalRowFilter predicates.
Problem
The in-memory Bigtable emulator ( bttest ) doesn't properly clean up row state after per-cell filtering (e.g., FamilyNameRegexFilter ). When a filter removes all cells from a family, the family entry and its empty columns remain as "ghost" entries in the row's internal data structures. This causes ConditionalRowFilter predicates to evaluate incorrectly when combined with FamilyNameRegexFilter in a chain — returning 0 results where production Bigtable returns the expected data.
Additionally, row.copy() shares the colNames slice between original and copy, which can lead to data corruption when filters modify column lists on copied rows (as happens during ConditionalRowFilter predicate evaluation).
Changes
File: inmem.go
1. Clean up empty entries after per-cell filtering (~line 897)
After the per-cell filtering loop, remove columns with no remaining cells from the cells map, rebuild colNames , and delete families that have no cells left. This matches production Bigtable behavior where filter output only contains families/columns that actually have data.
Root Cause
1. filterRow handles FamilyNameRegexFilter via the per-cell path — it calls filterCells per column which sets non-matching cells to nil, but never removes the empty entries from the family's cells map or colNames . When ConditionalRowFilter later calls r.copy() and evaluates its predicate, the copy carries these ghost families, potentially causing incorrect predicate results in complex filter chains.
2. row.copy() assigned colNames: fam.colNames (a shared slice reference). If any operation on the copy triggers an append to colNames that fits within the existing capacity, it would silently corrupt the original row's column ordering.
Testing
The fix can be validated with a test like:
go
func TestConditionalRowFilterWithFamilyNameRegex(t *testing.T) {
// Setup: row with two families "rcsa" and "other"
// Filter: Chain [
// ConditionalRowFilter(
// predicate = FamilyNameRegexFilter("rcsa"),
// true_filter = PassAllFilter,
// false_filter = BlockAllFilter,
// ),
// CellsPerColumnLimitFilter(1),
// ]
// Expect: row is returned with all cells (predicate matches "rcsa" family)
}