Closed iajoiner closed 1 month ago
Thanks for adding this, should be straightforward enough to add it to the column types and then fill everything out. A good strategy is to do a grep/search in the repo for something like "SmallInt" to find all of the places where it will need to be added.
/bounty $100
/attempt #154
with your implementation plan. Note: we will only assign an issue if you include an implementation plan with a time estimate. Additionally, to be assigned an issue, you must have previously contributed to the project. You can still work on an issue and submit a PR without being assigned./claim #154
in the PR body to claim the bountyThank you for contributing to spaceandtimelabs/sxt-proof-of-sql!
/attempt #154
@c0d33ngr please feel free to reach out with any questions, there are some nuances to this change that might warrant clarification.
Yes, I'll if I need clarification
On Thu, Sep 26, 2024, 8:37 PM Dustin Ray @.***> wrote:
@c0d33ngr https://github.com/c0d33ngr please feel free to reach out with any questions, there are some nuances to this change that might warrant clarification.
— Reply to this email directly, view it on GitHub https://github.com/spaceandtimelabs/sxt-proof-of-sql/issues/154#issuecomment-2377784296, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZKEA4QCHGTMXZH6JOJ5SDTZYRPBHAVCNFSM6AAAAABODOZQCGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZXG44DIMRZGY . You are receiving this because you were mentioned.Message ID: @.***>
💡 @Ashu999 submitted a pull request that claims the bounty. You can visit your bounty board to reward.
/attempt #154
Algora profile | Completed bounties | Tech | Active attempts | Options |
---|---|---|---|---|
@onyedikachi-david | 7 bounties from 4 projects | JavaScript, Shell |
﹟168, ﹟163 |
Cancel attempt |
I added a clarification to the description that I many of these attempts need to handle.
:tada: This issue has been resolved in version 0.28.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:
🎉🎈 @Ashu999 has been awarded $100! 🎈🎊
Background and Motivation
We already have
SmallInt / i16
,Int / i32
, andBigInt / i64
data types supported. Adding an additionalTinyInt / i8
data type would be helpful to some users and would additionally have (minor) performance benefits with those columns.Changes Requested
TinyInt
variant to the various*Column*
enum types.TinyInt
everywhere thatSmallInt
,Int
, andBigInt
are handled.NOTE: be sure keep individual PRs small and self-contained. All net-new code should be tested within the PR.
EDIT: clarification
I've seen several attempts at this issue, and most have the following concern: there aren't any tests around using +, -, =, >, *, etc. on TinyInt columns.
In other words, I'm not convinced that the following query will execute properly if
a
,b
, andc
are TinyInt columns:It might be the case that there is code in place to do this. However, there should be unit tests providing evidence that this is the case.
At a minimum, in order to close this ticket we need to have at least one integration test showing end-to-end behavior with
TinyInt
columns and a non-trivial query that requires computation involvingTinyInt
.