Support DocumentSnapshot cursors #56

Activity

1. product-auto-label

   added

   api: firestoreIssues related to the googleapis/python-firestore API.Issues related to the googleapis/python-firestore API.

   on Jun 4, 2020
2. yoshi-automation

   added

   triage meI really want to be triaged.I really want to be triaged.

   on Jun 4, 2020
3. IlyaFaer

   added

   type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.‘Nice-to-have’ improvement, new feature or different behavior or design.

   and removed

   triage meI really want to be triaged.I really want to be triaged.

   on Jun 5, 2020
4. HemangChothani

   self-assigned this

   on Jun 5, 2020
5. HemangChothani commented on Jun 5, 2020

   

   HemangChothani

   on Jun 5, 2020

   Contributor

   More actions

   @schmidt-sebastian As per my research i think python supports DocumentSnapshot-based cursors, but the confusion is just because of naming convention, will update the name of the parameter so it could minimize the confusion.Let me know if i misunderstood anything or missing something.
   

   python-firestore/google/cloud/firestore_v1/query.py

   Line 460 in b1c5987

   def start_at(self, document_fields):

   
   

   python-firestore/google/cloud/firestore_v1/query.py

   Lines 475 to 480 in b1c5987

   	Args:
   	document_fields
   	(Union[:class:`~google.cloud.firestore_v1.document.DocumentSnapshot`, dict, list, tuple]):
   	a document snapshot or a dictionary/list/tuple of fields
   	representing a query results cursor. A cursor is a collection
   	of values that represent a position in a query result set.
6. schmidt-sebastian commented on Jun 5, 2020

   

   schmidt-sebastian

   on Jun 5, 2020

   Author

   More actions

   @HemangChothani Sorry, I missed this completely. Would it be possible to rename document_fields? That name caught my eyes and I immediately thought Python does not support these cursors.

   The reason I looked at this was to figure out whether the Python SDK considers "IN" filters to be equality cursors. I am not able to decipher the code to provide an answer (

   python-firestore/google/cloud/firestore_v1/query.py

   Line 627 in b1c5987

   if _has_snapshot_cursor:

   ). Only the operations GREATER_THAN, GREATER_THAN_OR_EQUALS, LESS_THAN or LESS_THAN_OR_EQUALS should be used to compute the implicit cursor. See also the fix here: googleapis/java-firestore#216. Do we need to port this fix?
7. HemangChothani commented on Jun 8, 2020

   

   HemangChothani

   on Jun 8, 2020

   Contributor

   More actions

   Would it be possible to rename document_fields?

   @schmidt-sebastian Yes, I have opened a PR #58

   Do we need to port this fix?

   I think "IN" filter is not considered as a equality cursors. If i compare the python code it is similar to the old java code, but still need a conformation.

   This line is not going to execute for == and array_contains operator.

   python-firestore/google/cloud/firestore_v1/query.py

   Lines 636 to 637 in b1c5987

   	if filter_.op in should_order and field not in order_keys:
   	orders.append(self._make_order(field, "ASCENDING"))

   For better view i think need to concern @crwilcox
8. HemangChothani commented on Jun 29, 2020

   

   HemangChothani

   on Jun 29, 2020

   Contributor

   More actions

   @crwilcox Any suggestion on Python SDK considers "IN" filters to be equality cursors?
9. tseaver

   added a commit that references this issue on Sep 1, 2020

   fix: name parameter to indicate snapshot support (#169)

   ...

   be98897
10. HemangChothani commented on Sep 2, 2020

    

    HemangChothani

    on Sep 2, 2020

    Contributor

    More actions

    @schmidt-sebastian I thinks this issue is resolved and only the operations GREATER_THAN, GREATER_THAN_OR_EQUALS, LESS_THAN or LESS_THAN_OR_EQUALS should be used to compute the implicit cursor.
    As we can see here:
    

    python-firestore/google/cloud/firestore_v1/base_query.py

    Lines 682 to 689 in be98897

    	if _has_snapshot_cursor:
    	should_order = [
    	_enum_from_op_string(key)
    	for key in _COMPARISON_OPERATORS
    	if key not in (_EQ_OP, "array_contains")
    	]
    	order_keys = [order.field.field_path for order in orders]
    	for filter_ in self._field_filters:

    so closing this issue for now, feel free to reopen if it doesn't fulfill the requirements.
11. HemangChothani

    closed this as completedon Sep 2, 2020
12. schmidt-sebastian commented on Sep 8, 2020

    

    schmidt-sebastian

    on Sep 8, 2020

    Author

    More actions

    Sounds good. Thanks for clarifying and sorry for the confusion.