Crash on dev ota server when requesting /records?min_target.version=53

[leplatrem]

$ http GET https://kinto-ota.dev.mozaws.net/v1/buckets/build-hub/collections/archives/records\?min_target.version\=53`

HTTP/1.1 503 Service Unavailable
Access-Control-Expose-Headers: Total-Records, Next-Page, Expires, Last-Modified, ETag, Content-Length, Pragma, Alert, Cache-Control, Retry-After, Backoff
Connection: keep-alive
Content-Length: 151
Content-Type: application/json
Date: Fri, 05 May 2017 15:13:28 GMT
Retry-After: 3
Server: nginx
X-Content-Type-Options: nosniff

{
    "code": 503, 
    "errno": 201, 
    "error": "Service Unavailable", 
    "message": "Service temporary unavailable due to overloading or maintenance, please retry later."
}

[Natim]

raise exceptions.BackendError(original=e) from e\nkinto.core.storage.exceptions.BackendError: DataError: (psycopg2.DataError) invalid input syntax for type numeric: "53.0b4"

[leplatrem]

Work around: put quotes around 53: /records?min_target.version="53"

[Natim]

Is that a feature we support or a side effect?

[leplatrem]

Is that a feature we support or a side effect?

What?

We want to use numeric comparison if both field and filter values are numeric, textual otherwise.

[Natim]

I was talking about the fact of adding quotes in the querystring.

[leplatrem]

It is not properly documented, so I guess we can classify as side effect :))

[Natim]

Do you think that something we should support?

[glasserc]

Refs #1216 and #1215. I think we're trying too hard to be clever with an ad-hoc string query syntax and IMO we should leave the current syntax in place ("simple" syntax, where anything that can be converted to a number is a number and everything else is a string) and add a robust syntax that can encode any value as well as its type. I'd propose JSON (so /records?min_target.version=53 for number, /records?min_target.version="53" for string, /records?min_target.version=null for null, etc.).

For comparison order, one reason I don't want to be too clever here is that I'm afraid of inventing an inconsistent order. E.g. 2.0 < 10 but 10 < "2.0b". Having a robust syntax doesn't completely avoid this problem because data in different records can be of different types, and we will have to order them somehow (if we get e.g. _sort=different_typed_field). How do we handle that? I don't think throwing an exception in this case is necessarily a bad thing, but if we have to invent an ordering, I think "all ints are less than all strings" [i.e. the Python2.7 comparison rules] is fine.

[Natim]

@glasserc I tried your idea of putting a JSON decoder there. But apparently it doesn't fix any of the issue we have. Because even if we decode the number as a number then if we do a Postgresql request with a numeric value on a row that contains string we still have the issue.

[glasserc]

There were two parts to this bug and the JSON decoder fixes the first one. Previously, this request would work "sometimes" depending on whether the data in the record looked numeric or not. Now, it never works, which is an improvement.

The second part of the bug is about how we compare numbers to strings. Like I wrote originally, we already have that problem. You can put record1 = {"flavor": "1"} and record2 = {"flavor": 1} and then _sort=flavor. What order comes out? Does Postgres let you do that? I think the answer to those questions should guide our fix.

[glasserc]

It seems that Postgres's sorting order is all strings, followed by all numbers:

{
    "data": [
        {
            "flavor": "1",
            "id": "1b210b24-b0e6-44c4-b427-e912a3c94697",
            "last_modified": 1496695080659,
            "null": null,
            "people": []
        },
        {
            "flavor": "2",
            "id": "417fa284-42d0-41ad-a39e-16f40542120c",
            "last_modified": 1496695076284,
            "null": null,
            "people": []
        },
        {
            "flavor": "strawberry",
            "id": "7f163dc4-4aad-4680-bb1d-0c8a4dbcf479",
            "last_modified": 1496694495121,
            "null": null,
            "people": []
        },
        {
            "flavor": 1,
            "id": "d41694ee-94a5-4f11-93c3-3b3cfee38e5a",
            "last_modified": 1496695085202,
            "null": null,
            "people": []
        },
        {
            "flavor": 2,
            "id": "83f1952a-5af8-4d98-95f3-93602adc3427",
            "last_modified": 1496695037864,
            "null": null,
            "people": []
        }
    ]
}

So I would say we should compare numbers as different from non-numerics, but compare them earlier.

[Natim]

Do we have a case where we mix string and integer in the database? Cannot we use the schema to make sure a fields is always of type string even if that string contains only numbers?

[leplatrem]

Cannot we use the schema to make sure a fields is always of type string even if that string contains only numbers?

The storage is not aware of the data schema

[glasserc]

This is fixed by #1258 (thanks @leplatrem for adding tests to confirm!).