Fix Decimal Match Logic

[yliuuuu]

Relevant Issues

• [Closes/Related To] N/A

Description

• Currently, PartiQL's decimal semantics remains vague.
• Based on the SQL semantics:

An exact numeric type has a precision P and a scale S. P is a positive integer that determines the number of significant digits in a particular radix R, where R is either 2 or 10. S is a non-negative integer. Every value of an exact numeric type of scale S is of the form n × 10^–S , where n is an integer such that –R^P ≤ n < R^P .

20) If a <precision> is omitted, then an implementation-defined <precision> is implicit.
21) The value of a <precision> shall be greater than 0 (zero). 
22) If a <scale> is omitted, then a <scale> of 0 (zero) is implicit.
23) If an <exact numeric type> contains <scale>, then the value of <scale> shall not be greater than the value of <precision> contained in that <exact numeric type>.

This PR fixes two things:

1. Precision of 0 will now throw a Semantic error

1.00 is decimal(0,0)
   | 
org.partiql.lang.eval.EvaluationException: Precision 0 should be a positive integer
	Semantic Error: at line 1, column 9: Invalid precision or scale for decimal, <UNKNOWN> : <UNKNOWN>

2. Fix constrained decimal matching logic:

Before:

PartiQL> 1.0000 is decimal(5,3)
   | 
===' 
true
--- 

Now:

PartiQL>  1.0000 is decimal(5,3)
   | 
===' 
false
--- 

Other Information

• Updated Unreleased Section in CHANGELOG: [YES/NO]
  • Yes.
• Any backward-incompatible changes? [YES/NO]
  • Yes. Bug fix for decimal matching, and forbid precision of 0.
  • < For this purpose, we define backward-incompatible changes as changes that—when consumed—can potentially result in
    errors for users that are using our public APIs or the entities that have public visibility in our code-base. >
• Any new external dependencies? [YES/NO]
  • No.

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[rchowell]

Thank you for the fix. Could you please add these tests to the conformance test suite?

[rchowell]

Also, was there an associated SQL feature number associated with SQL exact numeric types?

[yliuuuu]

Actually yes. This is a good call out. E011 : Numeric data types. E011-03 : DECIMAL and NUMERIC
data types.

[github-actions]

Conformance comparison report

	Base (7e1ffd4)	8d75cab	+/-
% Passing	97.42%	97.42%	0.00%
✅ Passing	4271	4271	0
❌ Failing	113	113	0
🔶 Ignored	0	0	0
Total Tests	4384	4384	0

Number passing in both: 4271

Number failing in both: 113

Number passing in Base (7e1ffd4) but now fail: 0

Number failing in Base (7e1ffd4) but now pass: 0

[codecov-commenter]

Codecov Report

Patch coverage: 83.33% and no project coverage change.

Comparison is base (7e1ffd4) 74.62% compared to head (8aa8073) 74.62%.

Additional details and impacted files

@@            Coverage Diff            @@
##               main     #974   +/-   ##
=========================================
  Coverage     74.62%   74.62%           
- Complexity     2056     2057    +1     
=========================================
  Files           241      241           
  Lines         17309    17315    +6     
  Branches       3037     3038    +1     
=========================================
+ Hits          12917    12922    +5     
- Misses         3553     3554    +1     
  Partials        839      839           

Flag	Coverage Δ
CLI	13.93% <ø> (ø)
EXAMPLES	80.67% <ø> (ø)
LANG	79.72% <83.33%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ql/lang/eval/visitors/PartiqlAstSanityValidator.kt	84.52% <83.33%> (-0.10%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.