Hi Julian,
Thank you for your reply. I understand now that "parsing" is the wrong term.
What is the correct term I should be using? "Converting"?
I am not sure that the user has any control of this behaviour using the type
system. Calcite is hard-coded to represent intervals in millisecond precision,
even though its own default type system says they should be microsecond
precision.
Can you give me any hints on the correct way to go about changing the
implementation to address this issue?
Thanks in advance for your help,
Mark.
________________________________
From: Julian Hyde <jhyde.apa...@gmail.com>
Sent: 04 March 2025 19:30
To: dev@calcite.apache.org <dev@calcite.apache.org>
Subject: Re: Question about scale after parsing day/time intervals to RexLiteral
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%7C7bae0ba09010458500ba08dd5b530db2%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638767134461583390%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=W6qQid4uSrlXDIP%2F%2F8hnfzHqC%2BIF1AMOxWkgurWvYfQ%3D&reserved=0<https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCALCITE-6752&data=05%7C02%7C%7C7bae0ba09010458500ba08dd5b530db2%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638767134461606971%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=CUKYlC4xIWoLtvqYT%2BZMnvzhj5Xf%2FtF%2FwHnTO%2FWmHEc%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%7C7bae0ba09010458500ba08dd5b530db2%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638767134461620686%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=NzixKiOzPm6MkdTDt%2BVpwIVLjoTw1JOvs2bfRU%2B8N9Q%3D&reserved=0<https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCALCITE-5919&data=05%7C02%7C%7C7bae0ba09010458500ba08dd5b530db2%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C638767134461633805%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=x37P7pejHKqjPBnGXCGSyaD5U%2F7EpnxF%2BwFPbK6To7k%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.
