Skip to content

Call BigDecimal(num) instead of BigDecimal.new(num) - #928

Merged
sodabrew merged 2 commits into
brianmario:masterfrom
sodabrew:bigdecimal_not_new
Dec 20, 2017
Merged

Call BigDecimal(num) instead of BigDecimal.new(num)#928
sodabrew merged 2 commits into
brianmario:masterfrom
sodabrew:bigdecimal_not_new

Conversation

@sodabrew

Copy link
Copy Markdown
Collaborator

Resolves #925

Turns out I was chasing a NULL pointer on cBigDecimal from statement.c
when I removed the class lookup from result.c, extern meant the compiler
did not flag an uninitialized value.
@sodabrew sodabrew added this to the 0.5.0 milestone Dec 20, 2017
@sodabrew
sodabrew merged commit c0ba3d3 into brianmario:master Dec 20, 2017
@sodabrew
sodabrew deleted the bigdecimal_not_new branch December 20, 2017 19:14
@yahonda

yahonda commented Dec 20, 2017

Copy link
Copy Markdown
Contributor

Thanks for the fix. I have verified warning: BigDecimal.new is deprecated disappears.

It would be appreciated the new version of mysql2 gem released to support Ruby 2.5 which is scheduled to release on 12/25.

Thanks again for the fix.

@sodabrew

sodabrew commented Dec 20, 2017 via email

Copy link
Copy Markdown
Collaborator Author

@yahonda

yahonda commented Dec 20, 2017

Copy link
Copy Markdown
Contributor

Sounds good. 0.4.x including this fix is fine for me.

@salimane

Copy link
Copy Markdown

I can backport the deprecation warning fix to 0.4.x as well in January, there no other issue on Ruby 2.5 besides this warning.

@sodabrew it seems the above has not happened yet

jonhinson pushed a commit to jonhinson/mysql2 that referenced this pull request Jan 18, 2020
Call BigDecimal(num) instead of BigDecimal.new(num) to be ready for
this deprecation in Ruby 2.6, and eliminate a deprecation warning now.

Reduce the number of extern classes - while developing this change,
I was chasing a NULL pointer on cBigDecimal from statement.c
because I removed the class lookup from result.c, but extern meant the
compiler did not flag it as an uninitialized value.
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.

3 participants