Can you change the commit message from 'There is no support for checked arithmetic' to 'Support checked arithmetic'.
Using conformance as the flag to enable this feature seems perfect as long as the flag is only applied in the front end (during parsing, validation, sql-to-rel), and never referenced during planning or rel-to-sql. The relational algebra should not have 'modes' (except those implied by the type system and the operators used in expressions). On Fri, Nov 22, 2024 at 11:12 AM Mihai Budiu <[email protected]> wrote: > > I want to merge https://github.com/apache/calcite/pull/4044 which adds > support for checked arithmetic. Please let me know if you have any comments > using any medium (email, JIRA, github). > > I am fairly confident in the core change, the only thing I am unsure of is > whether Conformance is the right place to store a bit indicating whether > checked arithmetic should be used or not. > > Thank you, > Mihai > > ________________________________ > From: Julian Hyde <[email protected]> > Sent: Friday, October 4, 2024 12:20 PM > To: [email protected] <[email protected]> > Subject: Re: Checked arithmetic > > > This will probably require implementing some new Expression methods in > > linq4j, since I don't think many of the checked arithmetic functions exist > > I would be surprised if new Expression methods are required; I think new > methods in SqlFunctions (involved via Expressions.call) will be sufficient. > While unchecked PLUS will translate to Java ‘+’, checked PLUS should > translate to a new method SqlFunctions.plusChecked(int, int). > > > On Oct 4, 2024, at 10:38 AM, Mihai Budiu <[email protected]> wrote: > > > > The visitor approach seems to be the most unintrusive. > > > > My plan is as follows: > > > > > > * > > Prototype a RelNode visitor that replaces unchecked arithmetic with checked > > arithmetic. The visitor will be parameterized by configuration indicating > > whether DECIMAL or INTEGER arithmetic operations should be checked. > > * > > This will probably require implementing some new Expression methods in > > linq4j, since I don't think many of the checked arithmetic functions exist > > * > > Test the visitor in our compiler, which requires checked arithmetic. We > > have tests imported from Postgres which fail because of this. > > * > > If this approach works, contribute the visitor to Calcite > > * > > Change the Calcite type system to include configuration options for checked > > arithmetic and insert the visitor in the compilation flow after validation > > > > Regarding the way checked arithmetic is implemented for DECIMAL and INTEGER > > types, I believe that these require different solutions. For INTEGER all > > operations are supposed to be checked, whereas for DECIMAL only CAST > > operations to DECIMAL results are supposed to be checked. > > > > Egveny, indeed, a checked result is supposed to work the way you describe. > > And I am pretty sure Calcite today does not do that. > > > > The semantics of arithmetic is important if one uses the > > PROJECT_REDUCE_EXPRESSIONS rule, but there may be cases where other program > > simplifications rely on the semantics of arithmetic. > > > > This approach would not work if there are already arithmetic expression > > evaluations performed during the SqlToRel conversion. > > > > I will copy some of this discussion in the JIRA issue. > > > > Mihai > > > > ________________________________ > > From: stanilovsky evgeny <[email protected]> > > Sent: Thursday, October 3, 2024 10:34 PM > > To: [email protected] <[email protected]> > > Subject: Re: Checked arithmetic > > > > Mihai, thanks for this discussion ! > > my opinion : select Integer.MAX_VALUE + 1 i.e select 2147483647 + 1 need > > to raise overflow exception > > and select 2147483647::bigint + 1 need to return correct result. > > > >> I don't know what the SQL standard says. I suspect that most DBMS use > >> checked arithmetic. Because, as you say, people use a DBMS for > >> transactions such as managing bank accounts. > >> > >> But some people want to use SQL as a high-performance distributed > >> data-centric computation language, and these people will probably want > >> unchecked arithmetic. > >> > >> So, I agree with you. Whether to use checked arithmetic should be a > >> property of the type system - probably a property of each type. So > >> someone could have, say, checked DECIMAL and unchecked INT32. If we > >> introduce checked types, they would remain unchecked in the default > >> type system, thus ensuring backwards compatibility. > >> > >> Implementing checked arithmetic would be hard. One way to do it would > >> be to have a checked version of each operator - similar to the > >> SAFE_xxx operators [ > >> https://issues.apache.org/jira/browse/CALCITE-5591 ] - and have a > >> visitor that switches built-in operators to the checked versions. > >> > >> Generating correct SQL for other dialects will be even harder. To > >> safely omit bounds-checking, we would need to know that the bounds on > >> the Calcite type are identical to the bounds of the underlying type. > >> > >> Julian > >> > >> > >> On Thu, Oct 3, 2024 at 2:23 PM Mihai Budiu <[email protected]> wrote: > >>> > >>> Hello all, > >>> > >>> What is the semantics of arithmetic overflow in SQL? > >>> I assumed that SQL is supposed to use checked arithmetic, but it seems > >>> like this behavior is configurable for some database systems. Having > >>> checked arithmetic seems to be the in the spirit of SQL to provide > >>> exact results. You don't want to use wrap-around arithmetic when you > >>> manage your bank account. > >>> > >>> For example, if you look at an operator like Multiply in > >>> RexImplTable.java: > >>> > >>> defineBinary(MULTIPLY, Multiply, NullPolicy.STRICT, "multiply"); > >>> > >>> dispatches to: > >>> > >>> /** > >>> * A multiplication operation, such as (a * b), without > >>> * overflow checking, for numeric operands. > >>> */ > >>> Multiply(" * ", false, 3, false), > >>> > >>> This suggests that Calcite adopts unchecked arithmetic. And indeed, > >>> today Calcite ignores arithmetic overflows when evaluating expressions. > >>> Moreover, since it uses Java as a runtime, and Java has no arithmetic > >>> on Short or Byte, all computations are done using integer or long. > >>> Which means that lots of potential overflows are completely ignored. I > >>> have fixed this problem recently for Decimals, but the problem persists > >>> for all integer types. > >>> > >>> Ideally whether arithmetic is checked or not should be a property of > >>> the type system. > >>> However, this will make the implementation quite complex, since there > >>> are lots of places where Calcite generates arithmetic expressions. > >>> > >>> I think this is a long-standing bug in Calcite, which I'd like to fix. > >>> But what is the right solution? > >>> I have filed a related issue: > >>> https://issues.apache.org/jira/browse/CALCITE-6379 > >>> > >>> Mihai > >>> >
