bttest: CheckAndMutateRow unable to handle BlockAllFilter

[uschen]

due to:

	// Figure out which mutation to apply.
	whichMut := false
	if req.PredicateFilter == nil {
		// Use true_mutations iff row contains any cells.
		whichMut = !r.isEmpty()
	} else {
		// Use true_mutations iff any cells in the row match the filter.
		// TODO(dsymonds): This could be cheaper.
		nr := r.copy()
		filterRow(req.PredicateFilter, nr)
		whichMut = !nr.isEmpty()
	}

// filterRow modifies a row with the given filter. Returns true if at least one cell from the row matches,
// false otherwise. If a filter is invalid, filterRow returns false and an error.
func filterRow(f *btpb.RowFilter, r *row) (bool, error) {
	if f == nil {
		return true, nil
	}
	// Handle filters that apply beyond just including/excluding cells.
	switch f := f.Filter.(type) {
	case *btpb.RowFilter_BlockAllFilter:
		return !f.BlockAllFilter, nil
	case *btpb.RowFilter_PassAllFilter:
		return f.PassAllFilter, nil

return !f.BlockAllFilter, nil will not make row empty

and filterRow(req.PredicateFilter, nr) doesn't check bool nor error..............

PS: is it true other lang's client tests against bttest? if that's the case, why this is not discovered?

[uschen]

fix:

		ok, err := filterRow(req.PredicateFilter, nr)
		if err != nil {
			return nil, err
		}
		whichMut = ok && !nr.isEmpty()

[uschen]

and this needs to check bool and error too 😪

		for _, sub := range f.Interleave.Filters {
			sr := r.copy()
			filterRow(sub, sr)
			srs = append(srs, sr)
		}

[odeke-em]

Hello @uschen, thank you for reporting this issue! Apologies for the late reply!

So, that looks plausible that that missing check from filterRow could be a problem and I too found that a missing check was a cause of a bug in be4b3b2 and left a TODO. Let me examine how to firstly reproduce the problem. If you have a sample in your target language please let me know, otherwise I'll still work out one in proto structs.

PS: is it true other lang's client tests against bttest? if that's the case, why this is not discovered?

I believe so, yes, the Bigtable emulator is used by folks with other language/implementation clients, instead of hitting up the production servers.

[odeke-em]

I mailed out CL https://code-review.googlesource.com/c/gocloud/+/42170 which when it gets approved and merged, it will resolve this issue.