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

Types: Adjust CrateJsonEncoder to cast Decimal values to Python float types #604

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amotl
Copy link
Member

@amotl amotl commented Jan 16, 2024

About

This patch may be needed to fix value marshalling behaviour for Decimal types. It is apparently needed to be compatible with PostgreSQL.

This came up when using the SQLAlchemy dialect on behalf of a Singer/Meltano database sink component, in order to satisfy the software tests, where both implementation and test cases have been derived from the corresponding PostgreSQL adapter.

Status

Not sure if it classifies as a bug, but indeed it feels like a bug if the driver deviates from the standard DBAPI/PostgreSQL behaviour. Based on further investigations, the patch needs to find the right slot in one way or another.

References

Backlog

  • Adjust software tests.
  • Just a short note about type support within the documentation.
  • Changelog item. In this case, it is a BREAKING CHANGE, because, beforehand, values of Decimal type have been cast to Python's str type.

Beforehand, they have been marshalled to Python `str` type.
@amotl amotl changed the title Adjust CrateJsonEncoder to cast Decimal values to Python float types Types: Adjust CrateJsonEncoder to cast Decimal values to Python float types Jan 16, 2024
@seut
Copy link
Member

seut commented Jan 16, 2024

I think using a float won't work as a Decimal has a 128bit range and thus cannot fit into a float.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants