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> ------------------------------ ------------------------------
