change SQL numeric literals from float to decimal

[kmitchener]

trafficstars

Which issue does this PR close?

Closes #3394.

Rationale for this change

This introduces Decimal type in place of Float for numeric SQL literals.

What changes are included in this PR?

This PR:

• makes the change to Decimal128 from Float64
• consolidates the logic for converting the number string we get from the parser to Int64 or Decima128 types
• it also comments out tests that fail due to related bugs in the epic #3480

Are there any user-facing changes?

[kmitchener]

I want to highlight that I've entered issues (tracked in #3480) and commented out tests for some things that broke when using Decimal128. It does feel a bit like Decimal128 has some maturing to do to match the quality of the other numeric datatypes. But I wanted to submit this PR anyway -- if we're committed to making the change to decimal type in this way, then getting it in master is a good way to get additional eyes (and bigger brains than mine) on it.

[codecov-commenter]

Codecov Report

Merging #3488 (a700e7a) into master (9d028b3) will decrease coverage by 0.04%. The diff coverage is 93.18%.

@@            Coverage Diff             @@
##           master    #3488      +/-   ##
==========================================
- Coverage   85.75%   85.70%   -0.05%     
==========================================
  Files         299      299              
  Lines       55282    55227      -55     
==========================================
- Hits        47409    47335      -74     
- Misses       7873     7892      +19     

Impacted Files	Coverage Δ
datafusion/core/tests/parquet_pruning.rs	99.43% <ø> (ø)
datafusion/core/tests/sql/decimal.rs	100.00% <ø> (ø)
datafusion/core/tests/sql/expr.rs	99.86% <ø> (-0.01%)	:arrow_down:
datafusion/core/tests/sql/select.rs	99.78% <ø> (ø)
datafusion/core/tests/sql/subqueries.rs	94.95% <ø> (ø)
datafusion/sql/src/planner.rs	81.12% <90.00%> (+0.17%)	:arrow_up:
datafusion/core/tests/sql/aggregates.rs	99.35% <100.00%> (-0.03%)	:arrow_down:
datafusion/core/tests/sql/arrow_typeof.rs	100.00% <100.00%> (ø)
...sical-expr/src/aggregate/approx_percentile_cont.rs	69.64% <0.00%> (-11.91%)	:arrow_down:
datafusion/expr/src/binary_rule.rs	84.03% <0.00%> (-0.57%)	:arrow_down:
... and 6 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jimexist]

i think this change to UX is a bit too big. can we actually allow some of these functions to auto-convert from decimal to double or vice versa? i don't think asking existing queries to annotate ::double is helpful for adoption in the long term

[alamb]

I want to highlight that I've entered issues (tracked in https://github.com/apache/arrow-datafusion/issues/3480) and commented out tests for some things that broke when using Decimal128. It does feel a bit like Decimal128 has some maturing to do to match the quality of the other numeric datatypes. But I wanted to submit this PR anyway

THanks @kmitchener

I wonder if we can perhaps mark this PR as a draft as we work through whatever other maturing Decimal needs so that we don't require ::double. I agree with @Jimexist that that seems like a step backwards in functionality

[kmitchener]

Sure, I will switch it to draft and add another issue for changing the mathematical functions to accept and return decimal types in addition to float, where appropriate. That should reduce the amount of casting needed.

[alamb]

Thanks @kmitchener

[alamb]

Closing as this PR is over a year old. Please feel free to reopen it / rebase it if you plan to keep working on it