Repository: stac-utils/stac-fastapi (language: python)

Issue:
Sorting/paging inconsistencies and errors
Hello there,

I found an issue with searches, when sorting is used in combination with paging, especially when the a field that is a sort criterion is missing in some items. The result is not complete and can even be different depending on the sort order (ascending or descending). Items in the search result can even be duplicated.

The script below reproduces the problem (check the `endpoint` variable to make sure it uses the correct URL). It creates 10 very simple STAC items of which some have `eo:cloud_cover` values (with duplicates), but some items don't have that field.

```python
import requests
import datetime
from urllib.parse import urljoin


endpoint = "http://app-pgstac"
collection_name = "sort-test"


def create_test_collection():
    response = requests.delete(f"{endpoint}/collections/{collection_name}")
    body = {
        "type": "Collection",
        "id": collection_name,
        "stac_version": "1.0.0",
        "description": "Test collection to demonstrate sorting issues",
        "links": [
            { "rel": "root", "href": None, "type": "application/json" }
        ],
        "stac_extensions": [],
        "extent": {
            "spatial": { "bbox": [-180, -90, 180, 90] },
            "temporal": { "interval": ["2000-01-01T00:00:00Z", None] }
        }
    }
    requests.post(f"{endpoint}/collections", json=body)


def add_test_data():
    for i in range(10):
        cloud_cover = (i // 4) * 10
        now = datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")

        item = {
            "stac_version": "1.0.0",
            "type": "Feature",
            "id": "item-{0:02}".format(i + 1),
            "geometry": {
                "type": "Point",
                "coordinates": [12, 41]
            },
            "properties": {
                "datetime": "2022-01-02T03:04:05.678Z",
                "platform": "myplatform",
            },
            "assets": {},
            "links": [],
            "collection": collection_name
        }
        if i % 3 != 0:
            item["properties"]["eo:cloud_cover"] = cloud_cover

        requests.post(f"{endpoint}/collections/{collection_name}/items", json=item)


def search(body: dict):
    url = f"{endpoint}/search"
    if "limit" not in body: body["limit"] = 2

    received_items = []
    page = 1

    while True:
        print(f"Page {page}: POST {endpoint}: {body}")
        response = requests.post(url, json=body)

        if response.status_code >= 300:
            print(f"ERROR RESPONSE: {response.text}")
            return None

        result = response.json()
        for item in result["features"]:
            print(f"- ID: {item['id']}")
            received_items.append(item)
        print("----")

        next_link = next((link for link in result["links"] if "rel" in link and link["rel"] == "next"), None)

        if next_link:
            page += 1
            url = next_link["href"]
            if "body" in next_link:
                body = next_link["body"]
        else:
            break

    print(f"Summary ({len(received_items)} items):")
    for item in received_items:
        print("- ID {0} (eo:cloud_cover = {1})".format(item["id"], item["properties"]["eo:cloud_cover"] if "eo:cloud_cover" in item["properties"] else "--"))

    return received_items


create_test_collection()
add_test_data()

for sort_dir in ["asc", "desc"]:
    print(f"\n======== Sort direction: {sort_dir} ========\n")
    body = {
        "collections": [collection_name],
        "filter-lang": "cql2-json",
        "sortby": [
            { "field": "properties.eo:cloud_cover", "direction": sort_dir }
        ]
    }
    search(body=body)

print(f"\n======== Sort direction: asc + desc ========\n")
body = {
    "collections": [collection_name],
    "filter-lang": "cql2-json",
    "sortby": [
        { "field": "properties.eo:cloud_cover", "direction": "asc" },
        { "field": "id", "direction": "desc" }
    ]
}
search(body=body)
```

When making a search request for the whole collection (without filter) ordering by `eo:cloud_cover` and with a limit of 2 items per page, all 10 items should be returned (in 5 pages), but there are only 6 items (ascending sort) and 4 items (descending sort), respectively.

Furthermore. when sorting by two criteria in opposed directions, the second page (with the next token) fails with an PostgreSQL syntax error.

This is the output:


```

======== Sort direction: asc ========

Page 1: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}], 'limit': 2}
- ID: item-03
- ID: item-02
----
Page 2: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}], 'limit': 2, 'token': 'next:item-02'}
- ID: item-03
- ID: item-08
----
Page 3: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}], 'limit': 2, 'token': 'next:item-08'}
- ID: item-09
- ID: item-10
----
Summary (6 items):
- ID item-03 (eo:cloud_cover = 0)
- ID item-02 (eo:cloud_cover = 0)
- ID item-03 (eo:cloud_cover = 0)
- ID item-08 (eo:cloud_cover = 10)
- ID item-09 (eo:cloud_cover = 20)
- ID item-10 (eo:cloud_cover = --)

======== Sort direction: desc ========

Page 1: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2}
- ID: item-10
- ID: item-07
----
Page 2: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-07'}
- ID: item-04
- ID: item-01
----
Page 3: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-01'}
----
Summary (4 items):
- ID item-10 (eo:cloud_cover = --)
- ID item-07 (eo:cloud_cover = --)
- ID item-04 (eo:cloud_cover = --)
- ID item-01 (eo:cloud_cover = --)

======== Sort direction: asc + desc ========

Page 1: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}, {'field': 'id', 'direction': 'desc'}], 'limit': 2}
- ID: item-03
- ID: item-02
----
Page 2: POST http://app-pgstac: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}, {'field': 'id', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-02'}
ERROR RESPONSE: {"code":"PostgresSyntaxError","description":"syntax error at or near \")\""}
```

Has anybody else noticed issues like these? They are more likely to be found in large heterogeneous collections that might not be typical, but I think these are problems that should be fixed.

Thank you for any feedback!


Discussion / hints:
Minor issue in script, line 64 should read:
```python
        print(f"Page {page}: POST {url}: {body}")
```
What version of [pgstac](https://github.com/stac-utils/pgstac) are you running? There was a [release last week](https://github.com/stac-utils/pgstac/releases/tag/v0.6.13) that included some fixes for sorting and paging (e.g. https://github.com/stac-utils/pgstac/pull/154). If you're not on v0.6.13, can you update your **pgstac** and see if the issue persists? Thanks!
I was running 0.6.11. I upgraded to 0.6.13 and the result is slightly different. The syntax error for mixed ascending/descending sort no longer occurs, but when I run the test script again, the results are still inconsistent: 5 items (asc), 10 items (desc), and 6 items (asc + desc).

This is the output: 
```

======== Sort direction: asc ========

Page 1: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}], 'limit': 2}
- ID: item-03
- ID: item-02
----
Page 2: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}], 'limit': 2, 'token': 'next:item-02'}
- ID: item-03
- ID: item-08
----
Page 3: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}], 'limit': 2, 'token': 'next:item-08'}
- ID: item-09
----
Summary (5 items):
- ID item-03 (eo:cloud_cover = 0)
- ID item-02 (eo:cloud_cover = 0)
- ID item-03 (eo:cloud_cover = 0)
- ID item-08 (eo:cloud_cover = 10)
- ID item-09 (eo:cloud_cover = 20)

======== Sort direction: desc ========

Page 1: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2}
- ID: item-10
- ID: item-07
----
Page 2: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-07'}
- ID: item-04
- ID: item-01
----
Page 3: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-01'}
- ID: item-09
- ID: item-08
----
Page 4: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-08'}
- ID: item-06
- ID: item-05
----
Page 5: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-05'}
- ID: item-03
- ID: item-02
----
Summary (10 items):
- ID item-10 (eo:cloud_cover = --)
- ID item-07 (eo:cloud_cover = --)
- ID item-04 (eo:cloud_cover = --)
- ID item-01 (eo:cloud_cover = --)
- ID item-09 (eo:cloud_cover = 20)
- ID item-08 (eo:cloud_cover = 10)
- ID item-06 (eo:cloud_cover = 10)
- ID item-05 (eo:cloud_cover = 10)
- ID item-03 (eo:cloud_cover = 0)
- ID item-02 (eo:cloud_cover = 0)

======== Sort direction: asc + desc ========

Page 1: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}, {'field': 'id', 'direction': 'desc'}], 'limit': 2}
- ID: item-03
- ID: item-02
----
Page 2: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}, {'field': 'id', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-02'}
- ID: item-08
- ID: item-06
----
Page 3: POST http://app-pgstac/search: {'collections': ['sort-test'], 'filter-lang': 'cql2-json', 'sortby': [{'field': 'properties.eo:cloud_cover', 'direction': 'asc'}, {'field': 'id', 'direction': 'desc'}], 'limit': 2, 'token': 'next:item-06'}
- ID: item-05
- ID: item-09
----
Summary (6 items):
- ID item-03 (eo:cloud_cover = 0)
- ID item-02 (eo:cloud_cover = 0)
- ID item-08 (eo:cloud_cover = 10)
- ID item-06 (eo:cloud_cover = 10)
- ID item-05 (eo:cloud_cover = 10)
- ID item-09 (eo:cloud_cover = 20)
```
Another thing I noticed: it seems to be consistently the case (also with larger real-life data) that when sorting in **descending** order that the result is correct and complete (i.e. contains no duplicates and includes the items that don't have the property used as sort criterion), as in the second request above where all 10 items are returned).

And for the **ascending** sort, adding a second, unique sort criterion (e.g. `id`) makes sure that there are no longer duplicates and the result contains at least all items that have the property by which is sorted (6 with my sample data).

Thanks Pete for looking into this!
I've opened a PR with a failing test (based heavily on your example): #516. I'll work with the **pgstac** folks to see if we can't chase this down -- the error is probably over there.

Fix patch:
diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml
index 46aefd88e..9bad92ba9 100644
--- a/.github/workflows/cicd.yaml
+++ b/.github/workflows/cicd.yaml
@@ -11,11 +11,11 @@ jobs:
     strategy:
       matrix:
         python-version: ["3.8", "3.9", "3.10", "3.11"]
-    timeout-minutes: 10
+    timeout-minutes: 20
 
     services:
       db_service:
-        image: ghcr.io/stac-utils/pgstac:v0.6.13
+        image: ghcr.io/stac-utils/pgstac:v0.7.1
         env:
           POSTGRES_USER: username
           POSTGRES_PASSWORD: password
@@ -32,6 +32,7 @@ jobs:
           --health-interval 10s
           --health-timeout 10s
           --health-retries 10
+          --log-driver none
         ports:
           # Maps tcp port 5432 on service container to the host
           - 5432:5432
@@ -130,7 +131,7 @@ jobs:
         backend: ["sqlalchemy", "pgstac"]
     services:
       pgstac:
-        image: ghcr.io/stac-utils/pgstac:v0.6.13
+        image: ghcr.io/stac-utils/pgstac:v0.7.1
         env:
           POSTGRES_USER: username
           POSTGRES_PASSWORD: password
@@ -143,6 +144,7 @@ jobs:
           --health-interval 10s
           --health-timeout 5s
           --health-retries 5
+          --log-driver none
         ports:
           - 5432:5432
     steps:
@@ -176,7 +178,6 @@ jobs:
           POSTGRES_PORT: 5432
           PGUSER: username
           PGPASSWORD: password
-          PGHOST: localhost
           PGDATABASE: postgis
           APP_HOST: 0.0.0.0
           APP_PORT: 8080
@@ -187,41 +188,3 @@ jobs:
       - uses: actions/checkout@v3
       - name: Test generating docs
         run: make docs
-
-  docker-build-push:
-    runs-on: ubuntu-latest
-    needs: [test, validate, test-docs]
-    permissions:
-      contents: read
-      packages: write
-    strategy:
-      fail-fast: true
-      matrix:
-        backend: ["sqlalchemy", "pgstac"]
-    steps:
-      - name: Checkout repository
-        uses: actions/checkout@v3
-      - name: Log in to the Container registry
-        uses: docker/login-action@v2.1.0
-        with:
-          registry: ghcr.io
-          username: ${{ github.actor }}
-          password: ${{ secrets.GITHUB_TOKEN }}
-      - name: Extract metadata (tags, labels) for Docker
-        id: meta
-        uses: docker/metadata-action@v4.3.0
-        with:
-          images: ghcr.io/stac-utils/stac-fastapi
-          tags: |
-            type=schedule,suffix=-${{ matrix.backend }}
-            type=ref,event=branch,suffix=-${{ matrix.backend }}
-            type=ref,event=tag,suffix=-${{ matrix.backend }}
-            type=ref,event=pr,suffix=-${{ matrix.backend }}
-      - name: Build and push Docker image
-        uses: docker/build-push-action@v4.0.0
-        with:
-          context: .
-          file: docker/Dockerfile.${{ matrix.backend }}
-          push: true
-          tags: ${{ steps.meta.outputs.tags }}
-          labels: ${{ steps.meta.outputs.labels }}
diff --git a/.github/workflows/packages.yml b/.github/workflows/packages.yml
new file mode 100644
index 000000000..1988af0b2
--- /dev/null
+++ b/.github/workflows/packages.yml
@@ -0,0 +1,46 @@
+name: packages
+on:
+  push:
+    branches:
+      - master
+    tags:
+      - "*"
+
+jobs:
+  docker-build-push:
+    runs-on: ubuntu-latest
+    needs: [test, validate, test-docs]
+    permissions:
+      contents: read
+      packages: write
+    strategy:
+      fail-fast: true
+      matrix:
+        backend: ["sqlalchemy", "pgstac"]
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+      - name: Log in to the Container registry
+        uses: docker/login-action@v2.1.0
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+      - name: Extract metadata (tags, labels) for Docker
+        id: meta
+        uses: docker/metadata-action@v4.3.0
+        with:
+          images: ghcr.io/stac-utils/stac-fastapi
+          tags: |
+            type=schedule,suffix=-${{ matrix.backend }}
+            type=ref,event=branch,suffix=-${{ matrix.backend }}
+            type=ref,event=tag,suffix=-${{ matrix.backend }}
+            type=ref,event=pr,suffix=-${{ matrix.backend }}
+      - name: Build and push Docker image
+        uses: docker/build-push-action@v4.0.0
+        with:
+          context: .
+          file: docker/Dockerfile.${{ matrix.backend }}
+          push: true
+          tags: ${{ steps.meta.outputs.tags }}
+          labels: ${{ steps.meta.outputs.labels }}
diff --git a/CHANGES.md b/CHANGES.md
index bb265e77c..8bc597bfc 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -14,6 +14,7 @@
 * Updated CI to test against [pgstac v0.6.12](https://github.com/stac-utils/pgstac/releases/tag/v0.6.12) ([#511](https://github.com/stac-utils/stac-fastapi/pull/511))
 * Reworked `update_openapi` and added a test for it ([#523](https://github.com/stac-utils/stac-fastapi/pull/523))
 * Limit values above 10,000 are now replaced with 10,000 instead of returning a 400 error ([#526](https://github.com/stac-utils/stac-fastapi/pull/526))
+* Updated pgstac to v0.7.1 ([#535](https://github.com/stac-utils/stac-fastapi/pull/535))
 
 ### Removed
 
diff --git a/docker-compose.yml b/docker-compose.yml
index 506a4a37b..5637f7e0c 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -61,14 +61,13 @@ services:
 
   database:
     container_name: stac-db
-    image: ghcr.io/stac-utils/pgstac:v0.6.12
+    image: ghcr.io/stac-utils/pgstac:v0.7.1
     environment:
       - POSTGRES_USER=username
       - POSTGRES_PASSWORD=password
       - POSTGRES_DB=postgis
       - PGUSER=username
       - PGPASSWORD=password
-      - PGHOST=localhost
       - PGDATABASE=postgis
     ports:
       - "5439:5432"
diff --git a/docker/docker-compose.pgstac.yml b/docker/docker-compose.pgstac.yml
index 63514aab4..b95cba0bd 100644
--- a/docker/docker-compose.pgstac.yml
+++ b/docker/docker-compose.pgstac.yml
@@ -25,14 +25,13 @@ services:
       - pgstac
 
   pgstac:
-    image: ghcr.io/stac-utils/pgstac:v0.6.13
+    image: ghcr.io/stac-utils/pgstac:v0.7.1
     environment:
       - POSTGRES_USER=username
       - POSTGRES_PASSWORD=password
       - POSTGRES_DB=postgis
       - PGUSER=username
       - PGPASSWORD=password
-      - PGHOST=localhost
       - PGDATABASE=postgis
     ports:
       - "5439:5432"
diff --git a/docker/docker-compose.sqlalchemy.yml b/docker/docker-compose.sqlalchemy.yml
index 70d216305..f69516e4a 100644
--- a/docker/docker-compose.sqlalchemy.yml
+++ b/docker/docker-compose.sqlalchemy.yml
@@ -19,14 +19,13 @@ services:
       - pgstac
 
   pgstac:
-    image: ghcr.io/stac-utils/pgstac:v0.6.13
+    image: ghcr.io/stac-utils/pgstac:v0.7.1
     environment:
       - POSTGRES_USER=username
       - POSTGRES_PASSWORD=password
       - POSTGRES_DB=postgis
       - PGUSER=username
       - PGPASSWORD=password
-      - PGHOST=localhost
       - PGDATABASE=postgis
     ports:
       - "5439:5432"
diff --git a/stac_fastapi/pgstac/setup.py b/stac_fastapi/pgstac/setup.py
index 8664524d0..6632ff0c4 100644
--- a/stac_fastapi/pgstac/setup.py
+++ b/stac_fastapi/pgstac/setup.py
@@ -17,12 +17,12 @@
     "buildpg",
     "brotli_asgi",
     "pygeofilter>=0.2",
-    "pypgstac==0.6.*",
+    "pypgstac==0.7.*",
 ]
 
 extra_reqs = {
     "dev": [
-        "pypgstac[psycopg]==0.6.*",
+        "pypgstac[psycopg]==0.7.*",
         "pytest",
         "pytest-cov",
         "pytest-asyncio>=0.17",


Test patch:
diff --git a/stac_fastapi/pgstac/tests/api/test_api.py b/stac_fastapi/pgstac/tests/api/test_api.py
index e3baf32c1..ae766eb92 100644
--- a/stac_fastapi/pgstac/tests/api/test_api.py
+++ b/stac_fastapi/pgstac/tests/api/test_api.py
@@ -1,4 +1,5 @@
 from datetime import datetime, timedelta
+from typing import Any, Dict, List
 from urllib.parse import quote_plus
 
 import orjson
@@ -577,3 +578,47 @@ async def test_deleting_items_with_identical_ids(app_client):
         response = await app_client.get(f"/collections/{collection.id}/items")
         assert response.status_code == 200, response.json()
         assert not response.json()["features"]
+
+
+@pytest.mark.parametrize("direction", ("asc", "desc"))
+async def test_sorting_and_paging(app_client, load_test_collection, direction: str):
+    collection_id = load_test_collection.id
+    for i in range(10):
+        item = Item(
+            id=f"item-{i}",
+            geometry={"type": "Point", "coordinates": [-105.1019, 40.1672]},
+            bbox=[-105.1019, 40.1672, -105.1019, 40.1672],
+            datetime=datetime.now(),
+            properties={
+                "eo:cloud_cover": 42 + i if i % 3 != 0 else None,
+            },
+        )
+        item.collection_id = collection_id
+        response = await app_client.post(
+            f"/collections/{collection_id}/items",
+            json=item.to_dict(include_self_link=False, transform_hrefs=False),
+        )
+        assert response.status_code == 200
+
+    async def search(query: Dict[str, Any]) -> List[Item]:
+        items: List[Item] = list()
+        while True:
+            response = await app_client.post("/search", json=query)
+            json = response.json()
+            assert response.status_code == 200, json
+            items.extend((Item.from_dict(d) for d in json["features"]))
+            next_link = next(
+                (link for link in json["links"] if link["rel"] == "next"), None
+            )
+            if next_link is None:
+                return items
+            else:
+                query = next_link["body"]
+
+    query = {
+        "collections": [collection_id],
+        "sortby": [{"field": "properties.eo:cloud_cover", "direction": direction}],
+        "limit": 5,
+    }
+    items = await search(query)
+    assert len(items) == 10, items