Merge pull request #272 from redknightlois/RDBC-1026

RDBC-1026/1027/1028 Fix skip/take pagination, add and_also clause wrapping, fix suggest alias escaping

Author: poissoncorp

ravendb/documents/session/query.py

@@ -723,14 +723,18 @@ def _where_regex(self, field_name: str, pattern: str) -> None:
         where_token = WhereToken.create(WhereOperator.REGEX, field_name, parameter)
         tokens.append(where_token)
 
-    def _and_also(self) -> None:
+    def _and_also(self, wrap_previous_query_clauses: bool = False) -> None:
         tokens = self.__get_current_where_tokens()
         if not tokens:
             return
 
         if isinstance(tokens[-1], QueryOperatorToken):
             raise TypeError("Cannot add AND, previous token was already an operator token")
 
+        if wrap_previous_query_clauses:
+            tokens.insert(0, OpenSubclauseToken.create())
+            tokens.append(CloseSubclauseToken.create())
+
         tokens.append(QueryOperatorToken.AND())
 
     def _or_else(self) -> None:
@@ -923,7 +927,7 @@ def __build_pagination(self, query_text: List[str]) -> None:
             query_text.append(" limit $")
             query_text.append(self.__add_query_parameter(self._start or 0))
             query_text.append(", $")
-            query_text.append(self.__add_query_parameter(self._page_size or 0))
+            query_text.append(self.__add_query_parameter(self._page_size))
 
     def __build_include(self, query_text: List[str]) -> None:
         if (
@@ -2499,8 +2503,8 @@ def where_regex(self, field_name: str, pattern: str) -> DocumentQuery[_T]:
         self._where_regex(field_name, pattern)
         return self
 
-    def and_also(self) -> DocumentQuery[_T]:
-        self._and_also()
+    def and_also(self, wrap_previous_query_clauses: bool = False) -> DocumentQuery[_T]:
+        self._and_also(wrap_previous_query_clauses)
         return self
 
     def or_else(self) -> DocumentQuery[_T]:

ravendb/documents/session/tokens/query_tokens/definitions.py

@@ -22,7 +22,7 @@
 from ravendb.documents.session.tokens.query_tokens.query_token import QueryToken
 from ravendb.documents.session.utils.document_query import DocumentQueryHelper
 from ravendb.primitives.constants import VectorSearch
-from ravendb.tools.utils import Utils
+from ravendb.tools.utils import Utils, QueryFieldUtil
 
 
 class CompareExchangeValueIncludesToken(QueryToken):
@@ -992,7 +992,7 @@ def write_to(self, writer: List[str]) -> None:
             return
 
         writer.append(" as ")
-        writer.append(self.__alias)
+        writer.append(QueryFieldUtil.escape_if_necessary(self.__alias))
 
 
 class VectorSearchToken(WhereToken):

ravendb/tests/session_tests/test_query_clause_precedence.py

@@ -0,0 +1,105 @@
+"""
+DocumentQuery: and_also(wrap_previous_query_clauses=True) wraps preceding WHERE
+tokens in a subclause so AND has the correct precedence relative to OR.
+
+C# reference: FastTests/Client/Queries/QueryTests.cs
+  Query_CreateClausesForQueryDynamicallyWithOnBeforeQueryEvent
+"""
+
+from ravendb.tests.test_base import TestBase
+
+
+class Article:
+    def __init__(self, title: str = "", description: str = "", is_deleted: bool = False):
+        self.title = title
+        self.description = description
+        self.is_deleted = is_deleted
+
+
+class TestRavenDBAndAlsoWrapClauses(TestBase):
+    def setUp(self):
+        super().setUp()
+        with self.store.open_session() as session:
+            session.store(Article(title="foo", description="bar", is_deleted=False), "articles/1")
+            session.store(Article(title="foo", description="bar", is_deleted=True), "articles/2")
+            session.save_changes()
+
+    def test_and_also_accepts_wrap_previous_query_clauses_parameter(self):
+        """
+        C# spec: query.AndAlso(wrapPreviousQueryClauses: true) is a named parameter
+        that wraps preceding clauses in parentheses before appending AND.
+        and_also(wrap_previous_query_clauses=True) must be accepted without error.
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=Article)
+            q.and_also(wrap_previous_query_clauses=True)
+
+    def test_and_also_with_wrap_produces_subclause_rql(self):
+        """
+        C# spec: QueryTests.Query_CreateClausesForQueryDynamicallyWithOnBeforeQueryEvent
+          builds: search(Title, $p0) or search(Description, $p1)
+          then adds: andAlso(wrapPreviousQueryClauses: true).WhereEquals(IsDeleted, true)
+          expected RQL: "from 'Articles' where (search(Title, $p0) or search(Description, $p1)) and IsDeleted = $p2"
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=Article)
+            q = q.search("title", "foo")
+            q = q.or_else()
+            q = q.search("description", "bar")
+            q = q.and_also(wrap_previous_query_clauses=True)
+            q = q.where_equals("is_deleted", True)
+
+            rql = q.index_query.query
+            self.assertIn(
+                "(search(",
+                rql,
+                f"RQL should open subclause before search(), got: {rql!r}",
+            )
+            self.assertIn(
+                ") and ",
+                rql,
+                f"RQL should close subclause before AND, got: {rql!r}",
+            )
+            self.assertIn(
+                "is_deleted",
+                rql,
+                f"RQL should contain is_deleted after AND, got: {rql!r}",
+            )
+
+    def test_and_also_with_wrap_returns_one_filtered_result(self):
+        """
+        C# spec: expected results: 1 document (is_deleted=true only).
+        (search(title, foo) OR search(description, bar)) AND is_deleted=true
+        matches only articles/2.
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=Article)
+            q = q.search("title", "foo")
+            q = q.or_else()
+            q = q.search("description", "bar")
+            q = q.and_also(wrap_previous_query_clauses=True)
+            q = q.where_equals("is_deleted", True)
+            results = list(q)
+
+        self.assertEqual(
+            1,
+            len(results),
+            f"(title=foo OR description=bar) AND is_deleted=true should return 1 result, got {len(results)}",
+        )
+
+    def test_and_also_without_or_works(self):
+        """
+        and_also() works correctly when there is no preceding OR to wrap.
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=Article)
+            q = q.where_equals("title", "foo")
+            q = q.and_also()
+            q = q.where_equals("is_deleted", True)
+            results = list(q)
+
+        self.assertEqual(
+            1,
+            len(results),
+            "Simple AND (no preceding OR) should return exactly 1 result",
+        )

ravendb/tests/session_tests/test_query_pagination.py

@@ -0,0 +1,72 @@
+"""
+Query pagination: skip() without take() returns the expected documents.
+
+C# reference: FastTests/Issues/RavenDB_20542.cs
+  AddLongSkipToLINQ
+"""
+
+from ravendb.tests.test_base import TestBase
+
+
+class UserSkip:
+    def __init__(self, name: str = ""):
+        self.name = name
+
+
+class TestRavenDB20542(TestBase):
+    def setUp(self):
+        super().setUp()
+        with self.store.open_session() as session:
+            for name in ["AA", "BB", "CC"]:
+                session.store(UserSkip(name=name))
+            session.save_changes()
+
+    def test_skip_one_without_take_returns_remaining_documents(self):
+        """
+        C# spec: session.Query<User>().Skip(1).ToList() → 2 results (AA, BB, CC minus 1).
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=UserSkip)
+            q = q.skip(1)
+            results = list(q)
+
+        self.assertEqual(
+            2,
+            len(results),
+            f"skip(1) on 3 documents should return 2, but got {len(results)}",
+        )
+
+    def test_skip_with_max_long_returns_no_results(self):
+        """
+        C# spec: session.Query<User>().Skip(long.MaxValue).ToList() → 0 results.
+        Skipping past all documents returns an empty list.
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=UserSkip)
+            q = q.skip(9223372036854775807)
+            results = list(q)
+
+        self.assertEqual(
+            0,
+            len(results),
+            f"skip(long.MaxValue) should skip all documents and return 0, got {len(results)}",
+        )
+
+    # ------------------------------------------------------------------ #
+    #  Baseline: skip() combined with take()                              #
+    # ------------------------------------------------------------------ #
+
+    def test_skip_with_take_returns_correct_slice(self):
+        """
+        Baseline: skip(1).take(10) on 3 documents returns 2.
+        """
+        with self.store.open_session() as session:
+            q = session.advanced.document_query(object_type=UserSkip)
+            q = q.skip(1).take(10)
+            results = list(q)
+
+        self.assertEqual(
+            2,
+            len(results),
+            f"skip(1).take(10) on 3 documents should return 2, got {len(results)}",
+        )

ravendb/tests/session_tests/test_suggest_alias_escaping.py

@@ -0,0 +1,65 @@
+"""
+Suggestions: display names containing spaces are quoted in the generated RQL.
+
+C# reference: SlowTests/Issues/RavenDB_20673.cs
+  CustomizeDisplayNameWithSpaces, CustomizeDisplayNameWithOutSpaces
+"""
+
+from ravendb.documents.queries.suggestions import SuggestionBuilder
+from ravendb.tests.test_base import TestBase
+
+
+class User:
+    def __init__(self, name: str = None):
+        self.name = name
+
+
+class TestRavenDB20673(TestBase):
+    def setUp(self):
+        super().setUp()
+
+    def _setup_data(self):
+        with self.store.open_session() as session:
+            session.store(User(name="dan"), "users/1")
+            session.store(User(name="daniel"), "users/2")
+            session.store(User(name="danielle"), "users/3")
+            session.save_changes()
+
+        self.wait_for_indexing(self.store)
+
+    def test_suggestion_display_name_without_spaces(self):
+        """Display names without spaces must work — baseline sanity check."""
+        self._setup_data()
+
+        with self.store.open_session() as session:
+
+            def build(b: SuggestionBuilder):
+                b.by_field("name", "daniele").with_display_name("CustomizedName")
+
+            suggestion_query = session.query(object_type=User).suggest_using(build)
+            rql = suggestion_query.__str__()
+            self.assertIn("CustomizedName", rql)
+
+            results = suggestion_query.execute()
+            self.assertIn("CustomizedName", results)
+            self.assertEqual(2, len(results["CustomizedName"].suggestions))
+            self.assertIn("danielle", results["CustomizedName"].suggestions)
+
+    def test_suggestion_display_name_with_spaces(self):
+        """Display names containing spaces must be quoted in the RQL."""
+        self._setup_data()
+
+        with self.store.open_session() as session:
+
+            def build(b: SuggestionBuilder):
+                b.by_field("name", "daniele").with_display_name("Customized name with spaces")
+
+            suggestion_query = session.query(object_type=User).suggest_using(build)
+            rql = suggestion_query.__str__()
+            # escape_if_necessary wraps aliases containing spaces in single quotes
+            self.assertIn("'Customized name with spaces'", rql)
+
+            results = suggestion_query.execute()
+            self.assertIn("Customized name with spaces", results)
+            self.assertEqual(2, len(results["Customized name with spaces"].suggestions))
+            self.assertIn("danielle", results["Customized name with spaces"].suggestions)