I think the tests for Avatica pull from Calcite master to run, so until CALCITE-6282 and CALCITE-6322 are merged in the Calcite repo, the Avatica tests will fail.

On 28/03/2024 12:27 am, Mihai Budiu wrote:
I have tried to test these two calcite prs with both versions of avatica 1.23 
and 1.25
________________________________
From: Istvan Toth <st...@apache.org>
Sent: Wednesday, March 27, 2024 6:21:59 AM
To: dev@calcite.apache.org <dev@calcite.apache.org>
Subject: Re: Towards Avatica 1.25.0

Which is strange, because AFAIK one of the Avatica tests run by CI is
supposed to run the Calcite test suite with that Avatica build.

On Wed, Mar 27, 2024 at 12:17 PM Istvan Toth <st...@cloudera.com> wrote:

Based on my test run above, Calcite tests already fail with Avatica HEAD.
We should figure that out before adding new changes.

On Wed, Mar 27, 2024 at 2:35 AM Mihai Budiu <mbu...@gmail.com> wrote:

Ok, I have pushed two Calcite PRs that temporarily disable tests, once
these are accepted and merged I hope the Avatica PRs will also pass all
tests.

https://github.com/apache/calcite/pull/3708
https://github.com/apache/calcite/pull/3740

Mihai
________________________________
From: Francis Chuang <francischu...@apache.org>
Sent: Monday, March 25, 2024 9:48 PM
To: dev@calcite.apache.org <dev@calcite.apache.org>
Subject: Re: Towards Avatica 1.25.0

+1 I think this is a good plan

On 26/03/2024 3:25 pm, Mihai Budiu wrote:
Regarding
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6282
Avatica ignores time precision when returning TIME results

I have PR https://github.com/apache/calcite-avatica/pull/241 in
Avatica, and
a corresponding PR in Calcite
https://github.com/apache/calcite/pull/3740 which disables some tests.

I think that if we merge the Calcite one first, we can then merge the
Avatica one too, since the Avatica CI will succeed. Then as part of the new
Calcite release we mark the Bug.CALCITE_6282 as fixed and re-enable the
disabled tests.

Regarding
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6248
Illegal dates are accepted by casts

The situation is more complicatred, as I commented in the JIRA case.
I have an Avatica PR https://github.com/apache/calcite-avatica/pull/238.
While this does fix a genuine bug, more changes will be necessary to both
Avatica and Calcite to correctly handle TIME, DATE, and TIMESTAMP
conversions from strings in multiple contexts (literals, casts, values).
(Some of these bugs were fixed in Avatica 1.24, and some new bugs were
seemingly introduced in Avatica 1.24, but Calcite never upgraded to Avatica
1.24, so these bugs were not visible in Calcite.)

I expect we don't plan to fix all these bugs in Avatica 1.25, at least
I don't have time in the next week to work on fixing all of them. So my
proposal is to file a new issue for the remaining bugs that will continue
to exist in Avatica 1.25 (or maybe reuse one of the existing open issues),
and then merge my own PR 238 using a process similar to the one I described
above, using a Bug.CALCITE_6248.If you agree with this plan I will create
update the corresponding PR in Calcite for 6248.

Mihai

________________________________
From: Francis Chuang <francischu...@apache.org>
Sent: Monday, March 25, 2024 5:33 PM
To: dev@calcite.apache.org <dev@calcite.apache.org>
Subject: Re: Towards Avatica 1.25.0

On Avatica's side, we definitely want the tests to pass, otherwise it
will be hard to give the release a +1 during the voting process if there
are test failures.

On 26/03/2024 11:31 am, Mihai Budiu wrote:
Great. I will use this to test the changes on calcite's side. What
needs to happen to merge two prs in avatica? Do we need the avatica CI to
pass our can we temporarily ignore the failures?

If we need the CI to pass it looks like first we have to merge in
calcite main changes which temporarily disable all the tests that cause the
avatica ci to fail.

Mihai
________________________________
From: Istvan Toth <st...@apache.org>
Sent: Monday, March 25, 2024 2:14:54 AM
To: Istvan Toth <st...@apache.org>
Cc: dev@calcite.apache.org <dev@calcite.apache.org>
Subject: Re: Towards Avatica 1.25.0

I could not repro the compilation issue:

My workflow is:

In Avatica:
git checkout main
./gradlew publishToMavenLocal

In Calcite:
git checkout main
./gradlew clean build -Pcalcite.avatica.version=1.25.0-SNAPSHOT
-PenableMavenLocal

The Calcite test suite does fail, but everything compiles.

CalciteSqlOperatorTest > testExtractValue() STANDARD_ERROR
       [Fatal Error] :1:14: The markup in the document following the
root
element must be well-formed.
FAILURE  61.7sec,  492 completed,   3 failed,   1 skipped,
org.apache.calcite.test.CalciteSqlOperatorTest
FAILURE  63.2sec, 8900 completed,   6 failed, 101 skipped, Gradle Test
Run
:core:test

I'm pretty sure that this uses Avatica HEAD, because gradle will fail
early
if I specify a non-existent Avatica version.

Istvan

On Mon, Mar 25, 2024 at 9:52 AM Istvan Toth <st...@apache.org> wrote:

I have already approved
https://github.com/apache/calcite-avatica/pull/234

If Sergey is not available, any committer (including me) can merge it.

Istvan

On Mon, Mar 25, 2024 at 7:10 AM Mihai Budiu <mbu...@gmail.com> wrote:

I have authored the first two PRs in this list, they are certainly
ready
on the Avatica side, and they have been approved and are ready to
merge.

I have made corresponding PR on the Calcite side, and
I have been trying to test them with Calcite, but it's not easy.

First, there is a flag in Calcite called localAvatica, which is
supposed
to build using the a version of Avatica on the local disk. That
doesn't
work, because seemingly some packages have to be updated, including
gradle.

I have tried replacing the avatica-core and avatica-server jars in
the
gradle build files with local versions. But Calcite still doesn't
build:
some APIs have changed in Avatica, and Calcite will not build with
the new
APIs. In particular, the Avatica server Main class seems to require
different argument types.

Maybe there are other problems as well, but I got blocked on these.

Is it OK to merge the PRs in Avatica if the Avatica CI fails? The CI
fails because one of the tasks is to test the Calcite core, and
clearly
that will fail until Calcite itself is upgraded.

I could disable the failing tests in Calcite core temporarily, but I
suspect other Calcite projects will fail, which are not being tested
with
Avatica's CI.

I appreciate any help.
Mihai

________________________________
From: Francis Chuang <francischu...@apache.org>
Sent: Sunday, March 24, 2024 10:59 PM
To: dev@calcite.apache.org <dev@calcite.apache.org>
Subject: Towards Avatica 1.25.0

Hey everyone,

I want to start the discussion for releasing Avatica 1.25.0 before we
release Calcite 1.37.0.

Relevant discussions are here:
- Calcite 1.37.0:
https://lists.apache.org/thread/k27rwmhggmsbvwmgxs9fydcw2f0hook8
- Avatica PRs:
https://lists.apache.org/list?dev@calcite.apache.org:lte=1M:avatica

I think it would be a good idea to get these PRs in for the release:
- https://github.com/apache/calcite-avatica/pull/241
- https://github.com/apache/calcite-avatica/pull/238
- https://github.com/apache/calcite-avatica/pull/234

Community members, please take a look at those PRs and leave your
reviews if necessary. If possible, please consider merging as well.

I hope to make rc0 available for voting end of this week or early
next
week. Does this schedule suit?

Francis





--
*István Tóth* | Sr. Staff Software Engineer
*Email*: st...@cloudera.com
cloudera.com <https://www.cloudera.com>
[image: Cloudera] <https://www.cloudera.com/>
[image: Cloudera on Twitter] <https://twitter.com/cloudera> [image:
Cloudera on Facebook] <https://www.facebook.com/cloudera> [image:
Cloudera on LinkedIn] <https://www.linkedin.com/company/cloudera>
------------------------------
------------------------------

Reply via email to