-
Couldn't load subscription status.
- Fork 27
Adding decimal to documentation #242
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
Conversation
🚀 Build success!
Latest successful preview: https://preview-242--questdb-documentation.netlify.app/docs/
Commit SHA: 0632722
📦 Build generates a preview & updates link on each commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments, but super solid docs!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not factual! I think it has been broken for a while also on other rnd generators as not the first time I see this. But if you execute with 1 you will see it returns only some nan, not all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably this should be fixed in code, rather than on docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@puzpuzpuz I think the problem comes from the + 1 here, do you happen to know why it's there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of the initial reason to approach null rate like this in the random functions, but changing this would be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a breaking change for other types, the decimal type hasn't been released yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only pending point on my end to approve. If you agree this is not a decimal issue, but a problem with the current implementation, I am happy to approve
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation doesn't make sense with the current implementation, we should change one of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I am just not sure if the current behaviour is something we want to document, as I am not sure what it does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guess we can just approve, seeing as this is a wider problem and not related to decimal. The day (if any) it gets fixed for all other types, it will be fixed for decimal as well
Uh oh!
There was an error while loading. Please reload this page.
This PR adds the documentation to use the decimal type in QuestDB.
Related PR: #6068
TODO
Optional