Skip to content
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.

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

geo: higher precision for DefaultGeoJSONDecimalDigits #124175

Closed
wenyihu6 opened this issue May 14, 2024 · 3 comments · Fixed by #124312
Closed

geo: higher precision for DefaultGeoJSONDecimalDigits #124175

wenyihu6 opened this issue May 14, 2024 · 3 comments · Fixed by #124312
Assignees
Labels
A-cdc Change Data Capture C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-support Originated from a customer P-2 Issues/test failures with a fix SLA of 3 months T-cdc

Comments

@wenyihu6
Copy link
Contributor

wenyihu6 commented May 14, 2024

Is your feature request related to a problem? Please describe.

Currently, the number of decimal digits coordinates in GeoJSON is hardcoded at 9
via the DefaultGeoJSONDecimalDigits constant. A customer wants a higher precision
in this escalation https://github.com/cockroachlabs/support/issues/2946. Is it possible to increase the
default decimal digits or make this number configurable?

If this is not something we want to support, are there any workarounds for this?
I recommended adding a new column using the built in function ST_AsText(), but I
was wondering if there are better options here.

Jira issue: CRDB-38745

@wenyihu6 wenyihu6 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-queries SQL Queries Team labels May 14, 2024
@wenyihu6
Copy link
Contributor Author

I heard SQL queries is owning the geo package now. Feel free to re-direct if not.

@rharding6373
Copy link
Collaborator

rharding6373 commented May 16, 2024

I tested that const DefaultGeoJSONDecimalDigits = -1 allows arbitrary precision. I guessed this by reading the library source code. It's not documented anywhere AFAICT.

rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 16, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a precision, but now the json output
matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note: Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 16, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note: Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 16, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note: Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 17, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note: Do not limit precision when encoding geo data types to
JSON.
@wenyihu6 wenyihu6 added the O-support Originated from a customer label May 20, 2024
@wenyihu6 wenyihu6 added the A-cdc Change Data Capture label May 20, 2024
@blathers-crl blathers-crl bot added the T-cdc label May 20, 2024
Copy link

blathers-crl bot commented May 20, 2024

cc @cockroachdb/cdc

@wenyihu6 wenyihu6 added the P-2 Issues/test failures with a fix SLA of 3 months label May 20, 2024
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 21, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note(sql change): Do not limit precision when encoding geo data types to
JSON.
craig bot pushed a commit that referenced this issue May 21, 2024
124312: geo: allow arbitrary precision when encoding to json r=rharding6373 a=rharding6373

We used to set a default limit of 9 digit precision when converting geo datums to json. However, some users expect higher precision in json. This PR changes the default precision to -1, which allows arbitrary precision with our geo library. Because of truncation when the data enters the database, there is a limit to the precision, but now the json output matches the database data.

Epic: none
Fixes: #124175

Release note: Do not limit precision when encoding geo data types to JSON.

124412: jwtauthccl: add http client to jwt authenticator conf r=souravcrl a=souravcrl

Currently, we create a new http client every time we fetch the jwks URL or fetch jwk set from jwks URL. We can simply reuse the http client by adding it to authenticator conf struct.

fixes CRDB-38629
Epic None

Release note: None

124499: raft: panic on leader attempting to apply snapshot r=pav-kv a=nvanbenschoten

This commit replaces a warning and an attempt to gracefully handle a raft leader applying a snapshot with a panic. There's no use trying to handle this correctly when it is a bug that is not possible with the structure of the code.

Epic: None
Release note: None

Co-authored-by: rharding6373 <harding@cockroachlabs.com>
Co-authored-by: Sourav Sarangi <sourav.sarangi@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@craig craig bot closed this as completed in 9a99313 May 21, 2024
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 21, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note(sql change): Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 21, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note(sql change): Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 21, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note(sql change): Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 21, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note(sql change): Do not limit precision when encoding geo data types to
JSON.
rharding6373 added a commit to rharding6373/cockroach that referenced this issue May 22, 2024
We used to set a default limit of 9 digit precision when converting geo
datums to json. However, some users expect higher precision in json.
This PR changes the default precision to -1, which allows arbitrary
precision with our geo library. Because of truncation when the data
enters the database, there is a limit to the precision, but now the
json output matches the database data.

Epic: none
Fixes: cockroachdb#124175

Release note(sql change): Do not limit precision when encoding geo data types to
JSON.
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cdc Change Data Capture C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) O-support Originated from a customer P-2 Issues/test failures with a fix SLA of 3 months T-cdc
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants