You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Mark Lewis <ma...@outlook.com> on 2025/02/28 17:40:32 UTC
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.
Re: Question about scale after parsing day/time intervals to RexLiteral
Posted by Mark Lewis <ma...@outlook.com>.
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 <jh...@gmail.com>
Sent: 04 March 2025 19:30
To: dev@calcite.apache.org <de...@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 <ma...@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 <mb...@gmail.com>
> Sent: 28 February 2025 18:10
> To: dev@calcite.apache.org <de...@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 <ma...@outlook.com>
> Sent: Friday, February 28, 2025 9:40 AM
> To: dev@calcite.apache.org <de...@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.
Re: Question about scale after parsing day/time intervals to RexLiteral
Posted by Julian Hyde <jh...@gmail.com>.
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 <ma...@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 <mb...@gmail.com>
> Sent: 28 February 2025 18:10
> To: dev@calcite.apache.org <de...@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 <ma...@outlook.com>
> Sent: Friday, February 28, 2025 9:40 AM
> To: dev@calcite.apache.org <de...@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.
Re: Question about scale after parsing day/time intervals to RexLiteral
Posted by Mihai Budiu <mb...@gmail.com>.
The type system is rich enough to support nanosecond precision for time/timestamp.
So I think the right approach is BigDecimal for times - this can represent essentially all these types.
I expect that both long and decimal will be equally disruptive.
Mihai
________________________________
From: Mark Lewis <ma...@outlook.com>
Sent: Tuesday, March 4, 2025 3:48 AM
To: dev@calcite.apache.org <de...@calcite.apache.org>
Subject: Re: Question about scale after parsing day/time intervals to RexLiteral
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 <mb...@gmail.com>
Sent: 28 February 2025 18:10
To: dev@calcite.apache.org <de...@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 <ma...@outlook.com>
Sent: Friday, February 28, 2025 9:40 AM
To: dev@calcite.apache.org <de...@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.
Re: Question about scale after parsing day/time intervals to RexLiteral
Posted by Mark Lewis <ma...@outlook.com>.
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 <mb...@gmail.com>
Sent: 28 February 2025 18:10
To: dev@calcite.apache.org <de...@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 <ma...@outlook.com>
Sent: Friday, February 28, 2025 9:40 AM
To: dev@calcite.apache.org <de...@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.
Re: Question about scale after parsing day/time intervals to RexLiteral
Posted by Mihai Budiu <mb...@gmail.com>.
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://issues.apache.org/jira/browse/CALCITE-6752
See a list of related issues in the discussion for this other issue I have filed.
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 <ma...@outlook.com>
Sent: Friday, February 28, 2025 9:40 AM
To: dev@calcite.apache.org <de...@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.