googleapis/python-firestore

UserWarning: Detected filter using positional arguments.

XGeffrier opened this issue ยท 5 comments

Hello python-firestore team!
Thanks for this very useful lib and product.

A lot of my queries have recently triggered a UserWarning.

Minimal reproducible example:

import firebase_admin
from firebase_admin import credentials, firestore

# auth and instanciate client
cred = credentials.Certificate({...})
firebase_admin.initialize_app(cred)
db = firestore.client()

# perform simple "where" query
collection = db.collection("myCollection")
out = collection.where("field", "==", "value").get()

The final line triggers: UserWarning: Detected filter using positional arguments. Prefer using the 'filter' keyword argument instead.

It looks like it comes from this snippet, but I'm not skilled enough to investigate further.

I may use the filter keyword but I can't find any explanation in the doc on how to use it, and feel like I'm using the where method the way it was intended. Could you please either document the usage of filter keyword (and tell me what I'm doing wrong) or remove the warning if it should not be triggered?

Thanks a lot.

I had the same warning. There is no explanation about this in the documents and I couldn't find the filter usage in the changelogs as well. But the classes that are required can be found in the documents.

The filter argument takes BaseFilter class which is an abstract class and it is implemented in FieldFilter and BaseCompositeFilter. These classes can be imported from google.cloud.firestore_v1.base_query.

FieldFilter can be used to query against one field. Constructor of FieldFilter takes three positional arguments FieldFilter(field_path: str, op_string: str, value: Any | None = None) and it is the same arguments you use in where function.

BaseCompositeFilter can be used to combine or chain multiple fields to query. Constructor of BaseCompositeFilter takes two arguments BaseCompositeFilter(operator: Operator = StructuredQuery.CompositeFilter.Operator.OPERATOR_UNSPECIFIED, filters: Any | None = None). filters argument is a list of BaseFilter so it can contain both FieldFilter and BaseCompositeFilter (I am not exactly sure how it works when it contains BaseCompositeFilter but it works). The operator argument specifies how the filters are combined. The Operator class required for the operator argument is not implemented in the base_query and requires importing StructuredQuery from google.cloud.firestore_v1.types. You have two options for the operator argument StructuredQuery.CompositeFilter.Operator.AND and StructuredQuery.CompositeFilter.Operator.OR.

So, your simple "where" query can be written as:

# Import necessary classes
from google.cloud.firestore_v1.base_query import FieldFilter, BaseCompositeFilter
from google.cloud.types import StructuredQuery
# Create CollectionReference
collection_ref = db.collection("myCollection")
# Create FieldFilter
field_filter = FieldFilter("field1", "==", "value")
# Simple "where" query as in your example
query_on_single_field = collection_ref.where(filter=field_filter)

# Assume there are multiple fields you want to query on and you want to combine filters with OR operator. (Meaning it will combine BaseFilter's in filters list with OR and retrieve a document if any of the BaseFilter's matches the document)
composite_filter = BaseCompositeFilter(
   # If you use StructuredQuery.CompositeFilter.Operator.AND here it gives the same effect as chaining "where" functions
   operator=StructuredQuery.CompositeFilter.Operator.OR,
   filters=[
      FieldFilter("field1", "==", "value1"),
      FieldFilter("field2", "==", "value2")
   ]
)

# Query chaining multiple fields with OR
composite_query = collection_ref.where(filter=composite_filter)

To conclude, I don't think this feature is complete yet. It works when you use it but the naming and the importing style doesn't feel complete. There is another FieldFilter under StructuredQuery for example and if you use that it doesn't work. I also couldn't manage to create an instance of StructuredQuery or any of the classes defined with it. Also, if you take a look at the relevant section in the document it says "Snippet not available." for Python. So, I think at some point they will change how to query Firestore using Python but it is not ready yet. That is why even though I'm getting the same warning I decided not to use the filter keyword and just stick with the old where function.

Thanks a lot!

It's very good news that we now can combine queries with AND/OR operators, but I don't understand why the old-fashioned queries trigger a Warning. Unless these queries are soon deprecated, which should at least be mentioned in release notes and in documentation. If the intention is to keep it and maintain them, then the warning is useless.

I'm going to ignore the warnings just like you (on some of my scripts I run dozens of queries and have a warning each time), but I'm afraid to miss really important warnings hidden in the mass.

By reading the PR discussion (#698), I got the impression that the contributors just missed that point. Please contributors, fix it in a way or another (remove the warning, or explain why we should move to filters and document how if the feature is mature enough), I would be much grateful.

I'm still getting the warning when I use keywords:

xxx.where(field_path="abc", op_string="==", value="xyz")

still raises the warning:

UserWarning: Detected filter using positional arguments. Prefer using the 'filter' keyword argument instead."

Is the warning also raised for keyword arguments? Seems like a confusing message if so.

Also, are keyword/positional arguments expected to be deprecated? Is there some issue with using them? People have to go into their projects and make changes to deal with these warnings, so we should only be warned if there is good reason for it

kolea2 commented

Hi all, thanks for the feedback here! I recently created a PR to update the samples reflecting this change: GoogleCloudPlatform/python-docs-samples#10407.

As @XGeffrier mentions, this was introduced in #698, example https://github.com/googleapis/python-firestore/pull/698/files#r1147816611.

@GoktugCagiran - from your comment, "There is another FieldFilter under StructuredQuery for example and if you use that it doesn't work." could you explain this a bit more? Thank you!

@klanderson thanks for raising, will leave this issue open to investigate warnings when using keywords further.

@triplequark closed this as completed on Oct 18, 2023

@triplequark could you elaborate on whether any changes were made before closing the issue as completed (as opposed to not planned with commentary)?

And weigh in on @klanderson's questions:

Is the warning also raised for keyword arguments? Seems like a confusing message if so. Also, are keyword/positional arguments expected to be deprecated? Is there some issue with using them? People have to go into their projects and make changes to deal with these warnings, so we should only be warned if there is good reason for it