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