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

use absl for int128 #66

Merged
merged 6 commits into from
Mar 24, 2021
Merged

use absl for int128 #66

merged 6 commits into from
Mar 24, 2021

Conversation

filimonov
Copy link
Collaborator

@filimonov filimonov commented Nov 10, 2020

@azat
Copy link

azat commented Nov 25, 2020

@filimonov are you going to finish this?

@filimonov
Copy link
Collaborator Author

No :)

I've just make a backport from artpaul repo to expose it here / to sync the projects.

Have no idea what is broken and how to fix, and i know the code around very badly, feel free to pick it if you want. I guess some tiny efforts are needed to make it work.

@nshishov nshishov mentioned this pull request Dec 15, 2020
Enmk and others added 5 commits March 23, 2021 17:26
Added tests:
* Basic cases for Int128 numeric columns
* Parsing Decimal128 values from string (basic and with overflow)

Known limitations:
* Overflow checks are broken for std::numeric_limits<Int128>::min() when there is no __int128.
Tests for ItemView contructor
Fragile implementation of ItemView::ValidateData
And several other minor issues.
Fixes #66  to allow using absl::int128 as Int128
@Enmk Enmk merged commit 20dd563 into master Mar 24, 2021
@azat
Copy link

azat commented Mar 25, 2021

Does absl int128 has some benefits over simply gnu++17?
Requiring absl just for int128 looks a little bit overkill.

@Dlougach
Copy link

Dlougach commented Aug 6, 2021

I am a bit concerned about potential ODR problems if someone uses both absl and clickhouse-cpp in their projects.

@Enmk Enmk deleted the artpaul_119 branch February 1, 2022 12:26
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.

7 participants