Re: [DISCUSS] Towards Calcite 1.37.0

[Sergey Nuyanzin]

Thanks Benchao and Francis for pushing
you are right we need to finally make an RC

Thanks Mihai for driving this work
Starting tomorrow I will have more time which I can use to help with
review.
I would suggest to set a deadline like end of Saturday/begin of Sunday
in order to have an RC on Monday (IIRC anyway because of holidays voting
period was usually extended)

WDYT?




On Thu, Apr 18, 2024 at 6:55 PM Mihai Budiu  wrote:

> I have a few PR that I would like to land in 1.37 which have received no
> reviews.
> Three of them resolve bugs which I consider quite important.
> Let me describe them briefly, maybe this will help convince someone to
> contribute the effort of reviewing.
> Normally this kind of discussion is made inside of JIRA, but I noticed
> that if one is not following a particular JIRA ticket, they will never see
> the conversation.
>
> [CALCITE-6322] Casts to DECIMAL types are ignored<
> https://github.com/apache/calcite/pull/3733>
>
> Code like (CAST x AS DECIMAL(5, 2)) is treated by Calcite like a NOOP. I
> consider this as a major bug, which has been open for a very long time.
> This PR handles the cases where X is a numeric type.
>
> [CALCITE-2067] RexLiteral cannot represent accurately floating point
> values, including NaN, Infinity<
> https://github.com/apache/calcite/pull/3663>
>
> Calcite uses BigDecimal values to represent floating point literals. This
> has two problems:
>
>   *
> some FP constants cannot be represented exactly, leading to subtle
> rounding errors in computations that are performed at compilation-time
>   *
> FP literals cannot represent special FP values, like NaN
>
> This PR switches the representation of FP literals to use Java
> Double/Float values. This is a breaking change, because it changes the
> semantics of some programs. However, I claim that these programs were
> initially incorrect.
>
> [CALCITE-3522] Sql validator limits decimal literals to 64 bits and
> [CALCITE-6169] EnumUtils.convert does not implement the correct SQL cast
> semantics
>
> This PR solves two problems at once. I initially started by solving 3522,
> but then I realized that the solution uncovers many bugs in 6169, so I
> solved that one as well. Just days ago someone filed
> https://issues.apache.org/jira/browse/CALCITE-6366 for this very issue.
>
>
>   *
> 3522 solves the problem where Calcite limits DECIMAL literals to 64 bits.
> However, even the default DECIMAL implementation in Calcite supports 128
> bits, and some SQL dialects go even further (Postgres has billions of
> digits of precision). With this change there are no limits to the precision
> of a DECIMAL literal, and the limits come from the Calcite typeSystem in
> use.
>   *
> 6366 solves another problem related to the first issue described above,
> 6322, where narrowing casts (that convert a numeric value to a numeric
> value with fewer bits) do not report errors on overflow. This is another
> long-standing bug in Calcite.
>
> I will try to break this into two separate PRs that have to be merged in
> order; I will start with 6169. Maybe this will make it more palatable for
> the reviewers.
>
> Besides these 3 PRs, I have one more PR that I would like to land in 1.37,
> which is not a bugfix, but a new feature, so perhaps it's less urgent.
> [CALCITE-6071] RexCall should carry source position information for runtime
> error reporting is a
> relatively large PR, which adds source position information to RexCall and
> AggregateCall nodes. This is useful when a runtime error happens, like a
> division by 0. Using this information the runtime can report which​
> division in the program failed. Without this, debugging may be very
> difficult, especially when the program is large and can have many division
> operations, some hidden in operations like STDDEV_SAMP. This PR does not
> affect in any way the semantics of Calcite programs, it's a no-op for
> almost everyone. But it does touch many files, because it has to add new
> constructors for these classes and make sure that the information is
> available when the constructors are being invoked. At this point there are
> no users of this information in the Calcite codebase, but once the PR is
> merged we can use it even in the existing evaluator (that will also require
> significant work, since the evaluator itself does not expect source
> position information).
>
> Thank you,
> Mihai
>
> 
> From: Francis Chuang 
> Sent: Wednesday, April 17, 2024 9:05 PM
> To: dev@calcite.apache.org 
> Subject: Re: [DISCUSS] Towards Calcite 1.37.0
>
> I agree that it would be good to cut a release soon, as there haven't
> been too many commits in the last couple of days.
>
> I think it would be great for Sergey to set a deadline for the last
> commits to be accepted, close the main branch and start the release
> process. As Sergey is RM for the release, it would be best 

Re: [DISCUSS] Towards Calcite 1.37.0

[Mihai Budiu]

I have a few PR that I would like to land in 1.37 which have received no 
reviews.
Three of them resolve bugs which I consider quite important.
Let me describe them briefly, maybe this will help convince someone to 
contribute the effort of reviewing.
Normally this kind of discussion is made inside of JIRA, but I noticed that if 
one is not following a particular JIRA ticket, they will never see the 
conversation.

[CALCITE-6322] Casts to DECIMAL types are 
ignored

Code like (CAST x AS DECIMAL(5, 2)) is treated by Calcite like a NOOP. I 
consider this as a major bug, which has been open for a very long time. This PR 
handles the cases where X is a numeric type.

[CALCITE-2067] RexLiteral cannot represent accurately floating point values, 
including NaN, Infinity

Calcite uses BigDecimal values to represent floating point literals. This has 
two problems:

  *
some FP constants cannot be represented exactly, leading to subtle rounding 
errors in computations that are performed at compilation-time
  *
FP literals cannot represent special FP values, like NaN

This PR switches the representation of FP literals to use Java Double/Float 
values. This is a breaking change, because it changes the semantics of some 
programs. However, I claim that these programs were initially incorrect.

[CALCITE-3522] Sql validator limits decimal literals to 64 bits and 
[CALCITE-6169] EnumUtils.convert does not implement the correct SQL cast 
semantics

This PR solves two problems at once. I initially started by solving 3522, but 
then I realized that the solution uncovers many bugs in 6169, so I solved that 
one as well. Just days ago someone filed 
https://issues.apache.org/jira/browse/CALCITE-6366 for this very issue.


  *
3522 solves the problem where Calcite limits DECIMAL literals to 64 bits. 
However, even the default DECIMAL implementation in Calcite supports 128 bits, 
and some SQL dialects go even further (Postgres has billions of digits of 
precision). With this change there are no limits to the precision of a DECIMAL 
literal, and the limits come from the Calcite typeSystem in use.
  *
6366 solves another problem related to the first issue described above, 6322, 
where narrowing casts (that convert a numeric value to a numeric value with 
fewer bits) do not report errors on overflow. This is another long-standing bug 
in Calcite.

I will try to break this into two separate PRs that have to be merged in order; 
I will start with 6169. Maybe this will make it more palatable for the 
reviewers.

Besides these 3 PRs, I have one more PR that I would like to land in 1.37, 
which is not a bugfix, but a new feature, so perhaps it's less urgent. 
[CALCITE-6071] RexCall should carry source position information for runtime 
error reporting is a relatively 
large PR, which adds source position information to RexCall and AggregateCall 
nodes. This is useful when a runtime error happens, like a division by 0. Using 
this information the runtime can report which​ division in the program failed. 
Without this, debugging may be very difficult, especially when the program is 
large and can have many division operations, some hidden in operations like 
STDDEV_SAMP. This PR does not affect in any way the semantics of Calcite 
programs, it's a no-op for almost everyone. But it does touch many files, 
because it has to add new constructors for these classes and make sure that the 
information is available when the constructors are being invoked. At this point 
there are no users of this information in the Calcite codebase, but once the PR 
is merged we can use it even in the existing evaluator (that will also require 
significant work, since the evaluator itself does not expect source position 
information).

Thank you,
Mihai


From: Francis Chuang 
Sent: Wednesday, April 17, 2024 9:05 PM
To: dev@calcite.apache.org 
Subject: Re: [DISCUSS] Towards Calcite 1.37.0

I agree that it would be good to cut a release soon, as there haven't
been too many commits in the last couple of days.

I think it would be great for Sergey to set a deadline for the last
commits to be accepted, close the main branch and start the release
process. As Sergey is RM for the release, it would be best for him to
set the date as to when the main branch should be closed.

On 18/04/2024 12:55 pm, Benchao Li wrote:
> May I ask what's the status of releasing 1.37.0, since upgrading to
> Avatica 1.25 has been done 9 days ago, I would assume that there are
> no blockers now?
>
> I know many of us would like to clear some of the PR backlog before a
> release, however it would be better to have some balance between
> clearing PR backlog and rolling out releases regularly. It's been a
> bit more than 5 months since our last Calcite release (1.36.0 on
> 2023-11-10), 

Build failed in Jenkins: Calcite » Calcite-snapshots #869

[Apache Jenkins Server]

See 


Changes:

[mbudiu] [CALCITE-5289] Assertion failure in MultiJoinOptimizeBushyRule


--
[...truncated 273.71 KB...]
^
:176:
 warning: no @throws for java.io.IOException
  public String getPigScript(InputStream in, Map params)
^
:228:
 warning: no @return
  public List pigToSql(String pigQuery, SqlDialect sqlDialect)
  ^
:83:
 warning: no @param for config
  public static PigRelBuilder create(FrameworkConfig config) {
  ^
:83:
 warning: no @return
  public static PigRelBuilder create(FrameworkConfig config) {
  ^
:454:
 warning: no @param for flattenOutputAliases
  public RelBuilder multiSetFlatten(List flattenCols, List 
flattenOutputAliases) {
^
:608:
 warning: no @return
  public List getRelsForStores() {
   ^

> Task :redis:jandexMain
> Task :redis:processJandexIndex NO-SOURCE
> Task :redis:jar
> Task :redis:sourcesJar
> Task :redis:assemble

> Task :piglet:javadoc
:58:
 warning: no @return
  public static RelOptTable createRelOptTable(RelOptSchema schema,
^
:74:
 warning: no @param for config
  protected PigToSqlAggregateRule(Config config) {
^
21 warnings

> Task :redis:forbiddenApisMain
> Task :piglet:javadocJar

> Task :plus:compileJava
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

> Task :plus:classes
> Task :redis:compileTestJava
> Task :redis:testClasses
> Task :plus:jandexMain
> Task :plus:processJandexIndex NO-SOURCE
> Task :redis:forbiddenApisTest
> Task :redis:forbiddenApis
> Task :plus:jar
> Task :plus:sourcesJar
> Task :plus:assemble
> Task :redis:jandexTest
> Task :redis:jandex
> Task :plus:forbiddenApisMain
> Task :plus:compileTestJava
> Task :plus:testClasses
> Task :plus:forbiddenApisTest
> Task :plus:forbiddenApis
> Task :plus:jandexTest
> Task :plus:jandex
> Task :redis:javadoc
> Task :redis:javadocJar

> Task :piglet:test
  8.1sec, org.apache.calcite.test.PigletTest > 
testValues()
  8.1sec, org.apache.calcite.test.PigRelBuilderTest 
> testGroup2()
  9.8sec,   22 completed,   0 failed,   0 skipped, 
org.apache.calcite.test.PigRelBuilderTest

> Task :plus:javadoc
:98:
 warning: no @param for scaleFactor
  public TpcdsSchema(double scaleFactor) {
 ^
:120:
 warning: no @param for args
  public static void main(String[] args) {
 ^
2 warnings

> Task :plus:javadocJar

> Task :redis:test
WARNING   0.2sec,1 completed,   0 failed,   1 
skipped, org.apache.calcite.adapter.redis.RedisMiniServer

Gradle Test Executor 18 STANDARD_OUT
tc.testcontainers/ryuk:0.5.1 2024-04-18 16:37:33,559 
[ForkJoinPool-1-worker-1] INFO  - Pulling docker image: 
testcontainers/ryuk:0.5.1. Please be patient; this may take some time but only 
needs to be done once.
tc.testcontainers/ryuk:0.5.1 2024-04-18 16:37:33,999 
[docker-java-stream--1761815518] INFO  - Starting to pull image
tc.testcontainers/ryuk:0.5.1 2024-04-18 16:37:34,118 
[docker-java-stream--1761815518] INFO  - Pulling image layers:  0 pending,  0 
downloaded,  0 extracted, (0 bytes/0 bytes)
tc.testcontainers/ryuk:0.5.1 2024-04-18 16:37:34,226 
[docker-java-stream--1761815518] INFO  - Pulling image layers:  2 pending,  1 
downloaded,  0 extracted, (333 KB/? MB)

[jira] [Created] (CALCITE-6375) Fix failing quidem tests for FORMAT in CAST

[Jerin John (Jira)]

Jerin John created CALCITE-6375:
---

 Summary: Fix failing quidem tests for FORMAT in CAST
 Key: CALCITE-6375
 URL: https://issues.apache.org/jira/browse/CALCITE-6375
 Project: Calcite
  Issue Type: Bug
Reporter: Jerin John


CALCITE-2980 implemented the FORMAT clause used inside the CAST operator. We 
had imported a large number of quidem tests from Apache Impala for the same, 
stored in the file: 
[cast-with-format.iq|https://github.com/apache/calcite/blob/main/core/src/test/resources/sql/cast-with-format.iq].
 
Many of these tests are currently disabled as they're either unsupported or 
incorrectly implemented, some tests outputs may also need to be changed as per 
dialect behavior. Creating this placeholder issue to track and fix the pending 
tests.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

[jira] [Created] (CALCITE-6374) lattice throw npe when agg call covered with cast

[allendang (Jira)]

allendang created CALCITE-6374:
--

 Summary: lattice throw npe when agg call covered with cast 
 Key: CALCITE-6374
 URL: https://issues.apache.org/jira/browse/CALCITE-6374
 Project: Calcite
  Issue Type: Bug
  Components: core
Reporter: allendang
 Attachments: image-2024-04-18-21-44-04-262.png

!image-2024-04-18-21-44-04-262.png!



--
This message was sent by Atlassian Jira
(v8.20.10#820010)