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

streaming: ban double / decimal as pk #3863

Open
skyzh opened this issue Jul 14, 2022 · 7 comments
Open

streaming: ban double / decimal as pk #3863

skyzh opened this issue Jul 14, 2022 · 7 comments

Comments

@skyzh
Copy link
Contributor

skyzh commented Jul 14, 2022

Using them as PK might cause some keys not being successfully deleted.

@lmatz
Copy link
Contributor

lmatz commented Jul 14, 2022

Is it considered a bug, due to ser/deser or the decimal library we use?

@BowenXiao1999

This comment was marked as off-topic.

@skyzh
Copy link
Contributor Author

skyzh commented Jul 14, 2022

I think it is expected. Data values like decimal / double is not closed under addition. e.g.,

1.0 + 100000000000.0 - 100000000000.0 won't produce 1.0 due to precision issues. The same applies to decimal.

I guess it would make things wrong in some places.

@lmatz
Copy link
Contributor

lmatz commented Jul 14, 2022

Agree that double indeed needs to be banned, as we have no way of controlling these precision issues as long as we use the built-in double type in rust.

For decimal, I thought by its specification in PG, it is expected to be precise/exact.

My guess is that the current crate for decimal is quite limited due to its support for only 28 digits(scale? or something like that). If we would like to improve (e.g. by choosing another crate), it could be improved.

But still agree with banning it first, as decimal as part of pk should be a very rare case

@xiangjinwu
Copy link
Contributor

xiangjinwu commented Jul 14, 2022

The issue for real/float32 and double precision/float64 is that they are imprecise, making Eq and Hash less meaningful.

As for decimal (and also interval), it has another wired behavior: a == b does not mean f(a) == f(b). For example: 1.0 = 1.00 but 1.0::varchar <> 1.00::varchar. This was why we had different memcomparable encoding and value encoding.

@fuyufjh
Copy link
Member

fuyufjh commented Jul 15, 2022

Regarding to being user-friendly, this should be banned on the frontend side on certain queries, such as group by any real-type column.

@github-actions
Copy link
Contributor

This issue has been open for 60 days with no activity. Could you please update the status? Feel free to continue discussion or close as not planned.

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

No branches or pull requests

6 participants