Sorry to be pedantic but it’s important. To me, ‘parse’ means to go from text to an AST, without applying judgments such as type system. As far as I can tell, there is no problem with ‘parsing’. The parser produces an AST, specifically a SqlLiteral, with all of the digits that were in the SQL text.
The type system may choose to throw away some of those digits as it converts to a RexLiteral. But that is a decision that is under the user’s control. Julian > On Mar 4, 2025, at 3:48 AM, Mark Lewis <mark.s.le...@outlook.com> wrote: > > Hi Mihai, > > Thank you for the reply and the pointers to those existing issues. I think > the test failures I see when attempting different approaches to fixing the > behaviour are a combination of bad tests — tests that just test the output of > the current implementation, even if this is incorrect — and other parts of > the code that make similar assumptions about scale that need to be changed > alongside any change to the SQL to Rex conversion. > > Looking at the use of long data type to store the intervals, I wonder if this > would still be sufficient for day intervals to (the stated) microsecond > precision. I think that a signed long value should allow an interval of over > 106 million days. Even the maximum fractional second precision of 9 > (nanoseconds) would allow an interval of 106 thousand days to be represented. > A decimal would certainly be more flexible but maybe a long is sufficient > here, if it is a less disruptive change to the implementation? > > My gut feeling is that the best approach for me to take would be to have > these intervals stored as microseconds, as indicated by their current default > scale and fractional second precision. Given the number of tests and other > parts of the code that are likely to need to be reworked alongside this > change, I am wary of attempting anything without some prior agreement from > the maintainers on the right approach to take. > > Regards, > > Mark. > ________________________________ > From: Mihai Budiu <mbu...@gmail.com> > Sent: 28 February 2025 18:10 > To: dev@calcite.apache.org <dev@calcite.apache.org> > Subject: Re: Question about scale after parsing day/time intervals to > RexLiteral > > Indeed, the "Default" precision of 3 seems to be hardwired in several places > in Calcite for TIMESTAMPs and INTERVALs (and maybe for TIME intervals too). > > I have filed a related issue about the JavaTypeFactory: > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCALCITE-6752&data=05%7C02%7C%7Cb3b2a84b4d8e447683fd08dd5823441f%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638763630672738402%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=qrHXwDlyGf99r0ZKrBqFmpFMltZH4flhC1w0nj3E%2FZo%3D&reserved=0<https://issues.apache.org/jira/browse/CALCITE-6752> > > See a list of related issues in the discussion for this other issue I have > filed. > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCALCITE-5919&data=05%7C02%7C%7Cb3b2a84b4d8e447683fd08dd5823441f%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638763630672761579%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=c6y8DtJN85gstZ3okmO6aoQab%2F1VdArRtcj%2B8Idc3Z0%3D&reserved=0<https://issues.apache.org/jira/browse/CALCITE-5919> > > Fixing these end-to-end may be difficult. > > Fixing the literals may be the easy part, but many other layers may need > work, even if you get the literals to represent the values you want. As I > discuss in the second issue, probably using BigDecimal is the right approach. > > When you say it "breaks many tests", are the tests wrong? Then they should > break. > > Mihai > > ________________________________ > From: Mark Lewis <mark.s.le...@outlook.com> > Sent: Friday, February 28, 2025 9:40 AM > To: dev@calcite.apache.org <dev@calcite.apache.org> > Subject: Question about scale after parsing day/time intervals to RexLiteral > > Using Calcite to parse an SQL interval day seems to always result in a > RexLiteral with a scale of 6, but with the value a millisecond representation > of the interval. This is because SqlLiteral.getValueAs() always returns > milliseconds, and the default fractional second precision is 6. The resulting > RexLiteral scale confused me since the value of 6 suggested a microsecond > representation. > > I tried to change this behaviour in several ways: > > > 1. > Change the default fractional second interval precision to 3. > 2. > Change SqlNodeToRexConverterImpl.convertLiteral() to scale the value passed > to RexBuilder.makeIntervalLiteral() to microseconds in order to match the > target scale returned by > sqlIntervalQualifier.getFractionalSecondPrecision(rexBuilder.getTypeFactory().getTypeSystem()). > 3. > Change SqlNodeToRexConverterImpl.convertLiteral() so that it creates a new > SqlIntervalQualifier that matches the one associated with the SqlLiteral but > with a fractional second precision of 3 to indicate milliseconds. This has no > effect because RexBuilder.makeIntervalLiteral() creates the RexLiteral's > RelDataType by passing the interval qualifier to > SqlTypeFactoryImpl.createSqlIntervalType(), and the canonize() call that > happens there discards the fractional second precision — I guess by using an > interned value with default fractional second precision. > > While some of these approaches did produce a RexLiteral that looked more as I > expected, they all broke many Calcite tests. > > Is it expected that day/time intervals should always be represented in > milliseconds but also report a scale of 6? If not, can you suggest the right > approach to implement the correct behaviour? > > Thanks in advance for your help, > > Mark.
