Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

firestore: Query with '!=' operator for nil/NaN returns incorrect results #11639

Open
sanbiv opened this issue Feb 23, 2025 · 2 comments · May be fixed by #11649
Open

firestore: Query with '!=' operator for nil/NaN returns incorrect results #11639

sanbiv opened this issue Feb 23, 2025 · 2 comments · May be fixed by #11649
Assignees
Labels
api: firestore Issues related to the Firestore API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@sanbiv
Copy link

sanbiv commented Feb 23, 2025

Client

Firestore

Environment

go 1.24

Code and Dependencies

package main
import (
	"cloud.google.com/go/firestore"
	"context"
        "log"
)

func main() {
          client, err := firestore.NewClient(context.Background(), "myProjectId")
          if err != nil {
	        	log.Fatalf("error initializing firestore emulator client: %v", err)
          }
          q := client.Collection("example").Where("field", "!=", nil)
	  if docs, err := q.Documents(context.Background()).GetAll(); err == nil {
		      fmt.Printf("Found %d documents\n", len(docs))
	   } else {
		      log.Fatal(err.String())
	  }
}
go.mod
module modname

go 1.24.0

require (
   cloud.google.com/go/firestore v1.18.0
)

Expected behavior

The query should return all documents where "field" IS NOT NULL or "field" IS NOT NaN.

Actual behavior

The query behaves as if it were == nil, returning only documents where "field" IS NULL.

Additional context

This issue started after a recent commit that introduced support for != nil. The implementation does not properly map != nil and != NaN to their correct Firestore query representations (IS_NOT_NULL and IS_NOT_NAN).
PR: #11112

Proposed Fix

Modify the toProto function in PropertyPathFilter struct as follows:

func (f PropertyPathFilter) toProto() (*pb.StructuredQuery_Filter, error) {
	if err := f.Path.validate(); err != nil {
		return nil, err
	}
	if uop, ok := unaryOpFor(f.Value); ok {
		if f.Operator != "==" && !(f.Operator == "!=" && f.Value == nil) {
			return nil, fmt.Errorf("firestore: must use '==' when comparing %v", f.Value)
		}
		ref, err := fref(f.Path)
		if err != nil {
			return nil, err
		}
                // ------------------------------------- [START FIX]
  		if f.Operator == "!=" {
			if uop == pb.StructuredQuery_UnaryFilter_IS_NULL {
				uop = pb.StructuredQuery_UnaryFilter_IS_NOT_NULL
			} else if uop == pb.StructuredQuery_UnaryFilter_IS_NAN {
				uop = pb.StructuredQuery_UnaryFilter_IS_NOT_NAN
			}
		}
                // ------------------------------------- [END FIX]
		return &pb.StructuredQuery_Filter{
			FilterType: &pb.StructuredQuery_Filter_UnaryFilter{
				UnaryFilter: &pb.StructuredQuery_UnaryFilter{
					OperandType: &pb.StructuredQuery_UnaryFilter_Field{
						Field: ref,
					},
					Op: uop,
				},
			},
		}, nil
	}
	var op pb.StructuredQuery_FieldFilter_Operator
	switch f.Operator {
	case "<":
		op = pb.StructuredQuery_FieldFilter_LESS_THAN
	case "<=":
		op = pb.StructuredQuery_FieldFilter_LESS_THAN_OR_EQUAL
	case ">":
		op = pb.StructuredQuery_FieldFilter_GREATER_THAN
	case ">=":
		op = pb.StructuredQuery_FieldFilter_GREATER_THAN_OR_EQUAL
	case "==":
		op = pb.StructuredQuery_FieldFilter_EQUAL
	case "!=":
		op = pb.StructuredQuery_FieldFilter_NOT_EQUAL
	case "in":
		op = pb.StructuredQuery_FieldFilter_IN
	case "not-in":
		op = pb.StructuredQuery_FieldFilter_NOT_IN
	case "array-contains":
		op = pb.StructuredQuery_FieldFilter_ARRAY_CONTAINS
	case "array-contains-any":
		op = pb.StructuredQuery_FieldFilter_ARRAY_CONTAINS_ANY
	default:
		return nil, fmt.Errorf("firestore: invalid operator %q", f.Operator)
	}
	val, sawTransform, err := toProtoValue(reflect.ValueOf(f.Value))
	if err != nil {
		return nil, err
	}
	if sawTransform {
		return nil, errors.New("firestore: transforms disallowed in query value")
	}
	ref, err := fref(f.Path)
	if err != nil {
		return nil, err
	}
	return &pb.StructuredQuery_Filter{
		FilterType: &pb.StructuredQuery_Filter_FieldFilter{
			FieldFilter: &pb.StructuredQuery_FieldFilter{
				Field: ref,
				Op:    op,
				Value: val,
			},
		},
	}, nil
}
@sanbiv sanbiv added the triage me I really want to be triaged. label Feb 23, 2025
@product-auto-label product-auto-label bot added the api: firestore Issues related to the Firestore API. label Feb 23, 2025
@bhshkh
Copy link
Contributor

bhshkh commented Feb 24, 2025

@sanbiv, Thanks for finding the fix!
I'll be happy to review the PR if you can create one

@bhshkh bhshkh added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Feb 24, 2025
@sanbiv
Copy link
Author

sanbiv commented Feb 26, 2025

@bhshkh Hey, I submitted the PR #11649 . Did I miss anything, or should I just wait?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the Firestore API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
2 participants