Skip to content

Fixed a bug where floats in the database would not be serialized corr… #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 26, 2023
Merged

Fixed a bug where floats in the database would not be serialized corr… #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49132da
Fixed a bug where floats in the database would not be serialized corr…
Jul 13, 2023
2be5658
Updated the changelog
Jul 13, 2023
84d50ad
Extracted the render call into its own class
Jul 13, 2023
4566cc4
Merge branch 'main' into fix-decimal-error
vincentsarago Jul 17, 2023
17950cb
update from main
vincentsarago Jul 25, 2023
25840e2
add tests for numeric and decimal
vincentsarago Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Note: Minor version `0.X.0` update might break the API, It's recommended to pin
- `type` query parameter to filter collections based on their type (`Function` or `Table`)
- fixed a small bug in the `tipg_properties` SQL function where the bounds property was not properly transformed to 4326 (author @RemcoMeeuwissen, https://github.com/developmentseed/tipg/pull/87)
- handling functions that are interpreted as collections but lack parameters (author @jackharrhy, https://github.com/developmentseed/tipg/pull/96)
- fixed a bug where orjson could not properly serialize float values present in the properties of a feature (author @RemcoMeeuwissen, https://github.com/developmentseed/tipg/pull/89)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this line to mention that Decimal will be converted to string


### Added

Expand Down
14 changes: 8 additions & 6 deletions tests/fixtures/my_data.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ SELECT AddGeometryColumn('public','my_data','geom',4326,'GEOMETRY',2);
CREATE INDEX "my_data_geom_geom_idx" ON "public"."my_data" USING GIST ("geom");
ALTER TABLE "public"."my_data" ADD COLUMN "id" VARCHAR;
ALTER TABLE "public"."my_data" ADD COLUMN "datetime" TIMESTAMP;
INSERT INTO "public"."my_data" ("geom" , "id", "datetime") VALUES ('0103000020E6100000010000001B0000003670CC05599B25C03A92CB7F483F54408907944DB9F221C0D9CEF753E315544069D68681BE5B22C0355D864BD1145440984C2580F45C27C062327530C20754409CB396CA942C30C08E6EC42E50F05340F32225E11DCB30C07C98C2D614ED5340075F984C15FC30C0075F984C15EC53400AA1BD9D6AD732C03439A530F50B5440D8BFC6C0170533C00414E74C050F54407650100F7C0E33C0B199D586A60F5440A01BF45DE29634C0B61719B9F6295440838D3D254B5D35C0DC611EC044375440B8A6A26802F135C06705618A2C4154407CBD21E2CF3136C09B1B77FC844554402CD49AE61D3736C076711B0DE045544039117CFD650136C001AEC11005475440DC27DD0AB9C935C0F45E61C1344854406182187FE9BA35C03AF2E08A854854400736A0D273F130C050CF32FAA1625440ED137AA9497230C0441F419D576554401D9FC06CB06E2BC0B1930B183C745440017C2AECC5F92AC01E2006F67A7554401895D40968822AC0986E1283C07654405D44620EE0782AC0E00B92AC54765440FAACE2F3F95C27C0CDCE93B2275354400D2FBCF61DD226C0385BB99C044D54403670CC05599B25C03A92CB7F483F5440', '0', '2004-10-19 10:23:54');
INSERT INTO "public"."my_data" ("geom" , "id", "datetime") VALUES ('0103000020E61000000100000019000000984067B8143E3DC043C2B8B8F40B5440ACEF9DFAC14B3DC0E950B3BEBB0C544070CE88D2DE503DC01B2FDD24060D544034C8A112A4243DC064CC7707650E54409232AD9551103DC079704A40060F5440A630DBCBFBF43CC0E22ABE1BDF0F5440AC95A5A7DFA638C09E34007606325440FE987A2A9D7238C05D165F0DA5335440D1BF9E64C80A38C0FF6D3AC6DC3654409ACC3E07335D36C0578150C82C4454407CBD21E2CF3136C09B1B77FC8445544039117CFD650136C001AEC110054754401EA7E8482ECF35C07F6ABC7493485440DC27DD0AB9C935C0F45E61C134485440A2F3387764C135C09C775737A44754405526CE34BBDB34C047F7C465133854408DF37646C5EA33C0F10FDC85BE2754406D6485236BA431C08C72AF36460054403EE8D9ACFA9C30C07CF2B0506BEE5340F32225E11DCB30C07C98C2D614ED5340FE41CA2BA27737C016B27D9C8ABB5340C442AD69DEA137C05F07CE1951BA5340F9CBEEC9C30A38C07E078C8947C05340898D7238194D38C059C5B4D10CC45340984067B8143E3DC043C2B8B8F40B5440', '1', '2004-10-20 10:23:54');
INSERT INTO "public"."my_data" ("geom" , "id", "datetime") VALUES ('0103000020E61000000100000013000000C0155236C40A38C052F1FFE1D8C75340B244B5A16EC837C014EBB5CD0CC4534073D712F2414F37C0D3BCE3141DBD5340FE41CA2BA27737C016B27D9C8ABB5340A2728C64C30A38C03BFB4402D0B553400C6AB4D7723A3DC0BDA377861D82534058CA32C4B15E3DC062105839B48053402A2097D1F19641C0EAE96F4E58CC5340F0A7C64B379941C07F6ABC7493CC5340E11AE2531A8741C01F2670501DCE5340CED31A45F57241C03EC92059D3CF534009E08D47F1E83FC0EAC3384350F05340DFE755925F713EC036A2858243005440ACEF9DFAC14B3DC0E950B3BEBB0C544034C8A112A4243DC064CC7707650E5440F602E719D4063DC0AE877727A90F54400A68226C78FA3CC0234A7B832F105440A630DBCBFBF43CC0E22ABE1BDF0F5440C0155236C40A38C052F1FFE1D8C75340', '2', '2004-10-21 10:23:54');
INSERT INTO "public"."my_data" ("geom" , "id", "datetime") VALUES ('0103000020E610000001000000110000001B2CBE53855542C051F99E0E805D534049A5CD2EAE0644C03857A7D846865340462575029A0844C0A60A46257586534063B4EEABC4F943C08D992E511D8853409C72BC6BC5E843C0920AAB5C038A5340721D3749863342C03D0220C7DABA53402A2097D1F19641C0EAE96F4E58CC5340E11AE2531A8741C01F2670501DCE534068226C787A7541C0075F984C15D05340CED31A45F57241C03EC92059D3CF534048E17A14AE173DC06B2BF697DD8353400C6AB4D7723A3DC0BDA377861D825340A03E0335AD283FC0314A54553C6953409C6F1F2DEA1541C00EA6095E6A425340BEC11726532541C0BE9F1A2FDD405340EB51B81E853342C0302C67AA4C5A53401B2CBE53855542C051F99E0E805D5340', '3', '2004-10-22 10:23:54');
INSERT INTO "public"."my_data" ("geom" , "id", "datetime") VALUES ('0103000020E610000001000000110000000A4C8422590E46C0B656FB86F03B5340D5E76A2BF60F46C0075F984C153C5340FA28B2217F0346C0CE0A257ADB3D5340BEE6287052F545C01AA33BF2DF3F5340F25A937BB7D244C009CB92853C69534049A5CD2EAE0644C03857A7D84686534063B4EEABC4F943C08D992E511D88534034A2B437F8EA43C0F54A5986388A53409C72BC6BC5E843C0920AAB5C038A534050AF9465883342C0363B85F6B5605340D43E0032881142C02A5884BF7F5D5340F4FDD478E90641C007F01648504453409C6F1F2DEA1541C00EA6095E6A4253404E4E9C88873342C06DC6E4C7471E53403EDF52396E3443C0DC9EAF2DC7FD524044696FF0854143C032772D211FFC52400A4C8422590E46C0B656FB86F03B5340', '4', '2004-10-23 10:23:54');
INSERT INTO "public"."my_data" ("geom" , "id", "datetime") VALUES ('0103000020E6100000010000000D000000BBE9944235C347C0EBF06E7961EE52406ADE718A8EC447C0D122DBF97EEE5240942D6301ECB947C05B59871F60F0524086CAEEF61AAE47C0BDEF3BBB76F252400A4C8422590E46C0B656FB86F03B5340FA28B2217F0346C0CE0A257ADB3D534057EC2FBB27F745C02B1895D409405340BEE6287052F545C01AA33BF2DF3F53401D386744692743C07958A835CDFF52403EDF52396E3443C0DC9EAF2DC7FD5240B9E39237FD0645C0574B4E2543B552400AD7A3703D1245C03A234A7B83B35240BBE9944235C347C0EBF06E7961EE5240', '5', '2004-10-24 10:23:54');
ALTER TABLE "public"."my_data" ADD COLUMN "decimal" DECIMAL;
ALTER TABLE "public"."my_data" ADD COLUMN "numeric" NUMERIC;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added values for numeric and it seems that they get returned as Decimal so we're fine

INSERT INTO "public"."my_data" ("geom" , "id", "datetime", "decimal", "numeric") VALUES ('0103000020E6100000010000001B0000003670CC05599B25C03A92CB7F483F54408907944DB9F221C0D9CEF753E315544069D68681BE5B22C0355D864BD1145440984C2580F45C27C062327530C20754409CB396CA942C30C08E6EC42E50F05340F32225E11DCB30C07C98C2D614ED5340075F984C15FC30C0075F984C15EC53400AA1BD9D6AD732C03439A530F50B5440D8BFC6C0170533C00414E74C050F54407650100F7C0E33C0B199D586A60F5440A01BF45DE29634C0B61719B9F6295440838D3D254B5D35C0DC611EC044375440B8A6A26802F135C06705618A2C4154407CBD21E2CF3136C09B1B77FC844554402CD49AE61D3736C076711B0DE045544039117CFD650136C001AEC11005475440DC27DD0AB9C935C0F45E61C1344854406182187FE9BA35C03AF2E08A854854400736A0D273F130C050CF32FAA1625440ED137AA9497230C0441F419D576554401D9FC06CB06E2BC0B1930B183C745440017C2AECC5F92AC01E2006F67A7554401895D40968822AC0986E1283C07654405D44620EE0782AC0E00B92AC54765440FAACE2F3F95C27C0CDCE93B2275354400D2FBCF61DD226C0385BB99C044D54403670CC05599B25C03A92CB7F483F5440', '0', '2004-10-19 10:23:54', 1.25, 1.25);
INSERT INTO "public"."my_data" ("geom" , "id", "datetime", "decimal", "numeric") VALUES ('0103000020E61000000100000019000000984067B8143E3DC043C2B8B8F40B5440ACEF9DFAC14B3DC0E950B3BEBB0C544070CE88D2DE503DC01B2FDD24060D544034C8A112A4243DC064CC7707650E54409232AD9551103DC079704A40060F5440A630DBCBFBF43CC0E22ABE1BDF0F5440AC95A5A7DFA638C09E34007606325440FE987A2A9D7238C05D165F0DA5335440D1BF9E64C80A38C0FF6D3AC6DC3654409ACC3E07335D36C0578150C82C4454407CBD21E2CF3136C09B1B77FC8445544039117CFD650136C001AEC110054754401EA7E8482ECF35C07F6ABC7493485440DC27DD0AB9C935C0F45E61C134485440A2F3387764C135C09C775737A44754405526CE34BBDB34C047F7C465133854408DF37646C5EA33C0F10FDC85BE2754406D6485236BA431C08C72AF36460054403EE8D9ACFA9C30C07CF2B0506BEE5340F32225E11DCB30C07C98C2D614ED5340FE41CA2BA27737C016B27D9C8ABB5340C442AD69DEA137C05F07CE1951BA5340F9CBEEC9C30A38C07E078C8947C05340898D7238194D38C059C5B4D10CC45340984067B8143E3DC043C2B8B8F40B5440', '1', '2004-10-20 10:23:54', 0.8, 0.8);
INSERT INTO "public"."my_data" ("geom" , "id", "datetime", "decimal", "numeric") VALUES ('0103000020E61000000100000013000000C0155236C40A38C052F1FFE1D8C75340B244B5A16EC837C014EBB5CD0CC4534073D712F2414F37C0D3BCE3141DBD5340FE41CA2BA27737C016B27D9C8ABB5340A2728C64C30A38C03BFB4402D0B553400C6AB4D7723A3DC0BDA377861D82534058CA32C4B15E3DC062105839B48053402A2097D1F19641C0EAE96F4E58CC5340F0A7C64B379941C07F6ABC7493CC5340E11AE2531A8741C01F2670501DCE5340CED31A45F57241C03EC92059D3CF534009E08D47F1E83FC0EAC3384350F05340DFE755925F713EC036A2858243005440ACEF9DFAC14B3DC0E950B3BEBB0C544034C8A112A4243DC064CC7707650E5440F602E719D4063DC0AE877727A90F54400A68226C78FA3CC0234A7B832F105440A630DBCBFBF43CC0E22ABE1BDF0F5440C0155236C40A38C052F1FFE1D8C75340', '2', '2004-10-21 10:23:54', -5.68, -5.68);
INSERT INTO "public"."my_data" ("geom" , "id", "datetime", "decimal", "numeric") VALUES ('0103000020E610000001000000110000001B2CBE53855542C051F99E0E805D534049A5CD2EAE0644C03857A7D846865340462575029A0844C0A60A46257586534063B4EEABC4F943C08D992E511D8853409C72BC6BC5E843C0920AAB5C038A5340721D3749863342C03D0220C7DABA53402A2097D1F19641C0EAE96F4E58CC5340E11AE2531A8741C01F2670501DCE534068226C787A7541C0075F984C15D05340CED31A45F57241C03EC92059D3CF534048E17A14AE173DC06B2BF697DD8353400C6AB4D7723A3DC0BDA377861D825340A03E0335AD283FC0314A54553C6953409C6F1F2DEA1541C00EA6095E6A425340BEC11726532541C0BE9F1A2FDD405340EB51B81E853342C0302C67AA4C5A53401B2CBE53855542C051F99E0E805D5340', '3', '2004-10-22 10:23:54', 98, 98);
INSERT INTO "public"."my_data" ("geom" , "id", "datetime", "decimal", "numeric") VALUES ('0103000020E610000001000000110000000A4C8422590E46C0B656FB86F03B5340D5E76A2BF60F46C0075F984C153C5340FA28B2217F0346C0CE0A257ADB3D5340BEE6287052F545C01AA33BF2DF3F5340F25A937BB7D244C009CB92853C69534049A5CD2EAE0644C03857A7D84686534063B4EEABC4F943C08D992E511D88534034A2B437F8EA43C0F54A5986388A53409C72BC6BC5E843C0920AAB5C038A534050AF9465883342C0363B85F6B5605340D43E0032881142C02A5884BF7F5D5340F4FDD478E90641C007F01648504453409C6F1F2DEA1541C00EA6095E6A4253404E4E9C88873342C06DC6E4C7471E53403EDF52396E3443C0DC9EAF2DC7FD524044696FF0854143C032772D211FFC52400A4C8422590E46C0B656FB86F03B5340', '4', '2004-10-23 10:23:54', 7.55526, 7.55526);
INSERT INTO "public"."my_data" ("geom" , "id", "datetime", "decimal", "numeric") VALUES ('0103000020E6100000010000000D000000BBE9944235C347C0EBF06E7961EE52406ADE718A8EC447C0D122DBF97EEE5240942D6301ECB947C05B59871F60F0524086CAEEF61AAE47C0BDEF3BBB76F252400A4C8422590E46C0B656FB86F03B5340FA28B2217F0346C0CE0A257ADB3D534057EC2FBB27F745C02B1895D409405340BEE6287052F545C01AA33BF2DF3F53401D386744692743C07958A835CDFF52403EDF52396E3443C0DC9EAF2DC7FD5240B9E39237FD0645C0574B4E2543B552400AD7A3703D1245C03A234A7B83B35240BBE9944235C347C0EBF06E7961EE5240', '5', '2004-10-24 10:23:54', -78.56, -78.56);
ALTER TABLE public.my_data ADD COLUMN otherdt timestamptz;
ALTER TABLE public.my_data ADD COLUMN othergeom geometry;
UPDATE my_data SET otherdt=datetime+'1 year'::interval, othergeom=st_pointonsurface(geom);
Expand Down
4 changes: 4 additions & 0 deletions tests/routes/test_items.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,10 @@ def test_items_env_table_config_main(app, monkeypatch):
body = response.json()
assert body["features"][0]["geometry"]["type"] == "Polygon"

# Make sure that Postgres Decimal and Numeric are converted to str
assert isinstance(body["features"][0]["properties"]["decimal"], str)
assert isinstance(body["features"][0]["properties"]["numeric"], str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBD ☝️


response = app.get(
"/collections/public.my_data/items?datetime=2004-10-19T10:23:54Z"
)
Expand Down
1 change: 1 addition & 0 deletions tests/routes/test_tiles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def test_tilejson(app):
np.testing.assert_almost_equal(
resp_json["bounds"],
[-96.28961808496446, 46.11168980088226, -93.05330550250615, 48.56828559755232],
decimal=3,
)


Expand Down
25 changes: 24 additions & 1 deletion tipg/resources/response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,29 @@
"""tipg custom responses."""

from fastapi.responses import ORJSONResponse
import decimal
from typing import Any

import orjson

from fastapi.responses import JSONResponse


def default(obj):
"""Instruct orjson what to do with types it does not natively serialize"""
if isinstance(obj, decimal.Decimal):
return str(obj)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we use string instead of float here?

The result of this will return a string within the JSON so we loose the fact that it is a number in the table

cc @RemcoMeeuwissen @bitner

Copy link
Contributor

@bitner bitner Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually sure what we want here as best practice as decimal/numeric can be arbitrarily larger than what float could handle. Returning as numeric in the json would definitely fit most people's expectations, but could lose range / precision that is why people use those type rather than float.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah string felt like the safe option to me so that's why I went with it, but I don't know at which point precision is lost so I don't know if it is actually a relevant concern.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that we stick with string as it is the only way we can be certain to not be lossy. If someone knows their data fits into a float, they can alter their table.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at simplejson library, it seems they do convert decimal to float

a = {"a": decimal.Decimal('1.00000000006')}

simplejson.dumps(a)
'{"a": 1.00000000006}'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 I think it's because, simplejson will automatically convert decimal to float internally!

I'd say that we stick with string as it is the only way we can be certain to not be lossy. If someone knows their data fits into a float, they can alter their table.

👍 let's merge this, and we will revisit later if needed 🙏



class ORJSONResponse(JSONResponse):
"""Custom response handler for using orjson"""

def render(self, content: Any) -> bytes:
"""Render the content into a JSON response using orjson"""
return orjson.dumps(
content,
default=default,
option=orjson.OPT_NON_STR_KEYS | orjson.OPT_SERIALIZE_NUMPY,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we create a custom JSONResponse class to avoid duplicating the render code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah should be easy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Can you check if we need to do the same thing for postgres fields of type "numeric" as well (which is equivalent to the postgres "decimal" type?



class GeoJSONResponse(ORJSONResponse):
Expand Down