> 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 <mbu...@gmail.com> 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 <estanilovs...@gridgain.com> > Sent: Thursday, October 3, 2024 10:34 PM > To: dev@calcite.apache.org <dev@calcite.apache.org> > 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 <mbu...@gmail.com> 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 >>>
