If the flag is used only to enable an algebra-to-algebra transformation, that's a great model.
On Fri, Nov 22, 2024 at 1:46 PM Mihai Budiu <[email protected]> wrote: > > The flag is used right after SqlToRel, which was left unchanged. > Right now it's used in the Prepare class only, but the intent is for it to be > used right after SqlToRel to "correct" plans that are intended for a dialect > that requires checked arithmetic. > > I will change the title everywhere in a bit, let's see if there are more > comments. > > Mihai > > ________________________________ > From: Julian Hyde <[email protected]> > Sent: Friday, November 22, 2024 1:43 PM > To: [email protected] <[email protected]> > Subject: Re: Checked arithmetic > > 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 > > >>> > >
