Skip to content

Fix support for integer values in float/decimal type columns #2205

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
May 13, 2024
Merged

Fix support for integer values in float/decimal type columns #2205

merged 11 commits into from
May 13, 2024

Conversation

abhishekkumams
Copy link
Contributor

@abhishekkumams abhishekkumams commented May 8, 2024 •
edited
Loading

Why make this change?

What is this change?

  • using float.Parse() and decimal.Parse() which automatically takes care of int values.

How was this tested?

  • Added new test data points in the existing tests for insertion/updation of float/decimal/single type columns with Integer values

Sample Request(s)

Before:

image

image

After:

image

image

@abhishekkumams
Copy link
Contributor Author

/azp run

@abhishekkumams abhishekkumams self-assigned this May 8, 2024
@abhishekkumams abhishekkumams added the bug Something isn't working label May 8, 2024
@abhishekkumams abhishekkumams added this to the 1.1rc milestone May 8, 2024
@abhishekkumams
Copy link
Contributor Author

/azp run

@abhishekkumams abhishekkumams changed the title fix support for integer values in float/decimal type columns Fix support for integer values in float/decimal type columns May 8, 2024
@abhishekkumams
Copy link
Contributor Author

/azp run

seantleonard
seantleonard requested changes May 8, 2024
View reviewed changes
Copy link
Contributor

@seantleonard seantleonard left a comment

Choose a reason for hiding this comment

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

Before merging, would like additional perspective from you about your approach.

Aniruddh25
Aniruddh25 reviewed May 10, 2024
View reviewed changes
Aniruddh25
Aniruddh25 reviewed May 10, 2024
View reviewed changes
@abhishekkumams
Copy link
Contributor Author

/azp run

@ayush3797
Copy link
Contributor

Can you try with SINGLE_TYPE as well? Supply an int value for single types and see what happens?

@ayush3797
Copy link
Contributor

ayush3797 commented May 10, 2024 •
edited
Loading

And add tests for all
relevant types as well.

Also, please add tests for null values in the column if not already present.

@abhishekkumams
Copy link
Contributor Author

/azp run

@abhishekkumams
Copy link
Contributor Author

/azp run

@seantleonard seantleonard modified the milestones: 1.1rc, 1.1 Patch 01 May 11, 2024
@seantleonard
Copy link
Contributor

/azp run

Aniruddh25
Aniruddh25 approved these changes May 13, 2024
View reviewed changes
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for addressing all concerns.

@Aniruddh25 Aniruddh25 merged commit df426a4 into main May 13, 2024
@Aniruddh25 Aniruddh25 deleted the dev/abhishekkuma/fix-int-float-error branch May 13, 2024 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aniruddh25 Aniruddh25 Aniruddh25 approved these changes

@seantleonard seantleonard seantleonard approved these changes

@mbhaskar mbhaskar Awaiting requested review from mbhaskar

@Mathos1432 Mathos1432 Awaiting requested review from Mathos1432

@gledis69 gledis69 Awaiting requested review from gledis69

@aaronburtle aaronburtle Awaiting requested review from aaronburtle aaronburtle is a code owner

@tarazou9 tarazou9 Awaiting requested review from tarazou9

@ayush3797 ayush3797 Awaiting requested review from ayush3797

@aaronpowell aaronpowell Awaiting requested review from aaronpowell

@severussundar severussundar Awaiting requested review from severussundar

@ravishetye ravishetye Awaiting requested review from ravishetye

@rohkhann rohkhann Awaiting requested review from rohkhann

@neeraj-sharma2592 neeraj-sharma2592 Awaiting requested review from neeraj-sharma2592 neeraj-sharma2592 is a code owner

@sourabh1007 sourabh1007 Awaiting requested review from sourabh1007 sourabh1007 is a code owner

Assignees

@abhishekkumams abhishekkumams

Labels
bug Something isn't working
Projects
None yet
Milestone
1.1 Patch 01
Development

Successfully merging this pull request may close these issues.

Error when receiving Decimal value without a decimal present
5 participants
@abhishekkumams @ayush3797 @seantleonard @Aniruddh25 @aaronburtle