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

add Decimal type support #38

Closed
wants to merge 1 commit into from
Closed

Conversation

nikitka
Copy link
Contributor

@nikitka nikitka commented Nov 28, 2018

ClickHouse has supported Decimal type since 18.12.13 as experimental feature and since 18.14.9 as default feature. In this patch I've added support of Decimal type and simple test-cases that covers overflow and truncation behaviors.

Copy link
Owner

@xzkostyan xzkostyan left a comment

Choose a reason for hiding this comment

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

If you plan to support HTTP transport you should add converter here: clickhouse_sqlalchemy/drivers/http/transport.py:22 and escape decimal value here: clickhouse_sqlalchemy/drivers/http/escaper.py:8.

Also, bump clickhouse-server version in .travis.yml to the minimum required,

@nikitka
Copy link
Contributor Author

nikitka commented Nov 29, 2018

I see that you've already implement escape_decimal in escaper.py, but I don't understand correctly how to convert value in the transport. It would be nice to quantize value before we send it to the server (because application can have non default decimal rounding rules), but I don't know how to get precision and scale from the SQLA column declaration. Could you help me please with this?

@xzkostyan
Copy link
Owner

This is a good question. There is no column context during escaping process.

Okay. Let's bump clickhouse-server version and merge this PR.

@nikitka nikitka closed this Dec 14, 2018
@xzkostyan
Copy link
Owner

Hi. Your PR was merged into master.

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