I’ll add comments in the JIRA case.
> On Apr 9, 2022, at 11:32 PM, Raghav Sharma <[email protected]> > wrote: > > Hi Julian/ Stamatis! > > We have raised a PR and logged a JIRA ticket for the contribution of the > dialect file of Firebolt in Calcite. PFB the links for the same: > > 1. JIRA ticket link: https://issues.apache.org/jira/browse/CALCITE-5085 > 2. PR link: https://github.com/apache/calcite/pull/2764 > > Please take a look at the code, address the existing issues and add your > valuable comments. > > Currently the build fails due to one of the test cases(testSubstring) > inside ‘RelToSqlConverterTest.java’ file. Please check why does the > identifier quote string fails to be generated for the Firebolt dialect and > let us know how can we fix it. > > > Best, > Raghav > > > On Mon, 4 Apr 2022 at 4:26 PM Stamatis Zampetakis <[email protected]> wrote: > >> Hi Raghav, >> >> Some tests which make use of the TEST_DB property assume that the database >> has the foodmart dataset (possibly others as well) loaded [1]. >> If you need to run tests with Firebolt, Postgres, etc., you have to >> manually load the dataset there. To do that you can take some inspiration >> from calcite-test-dataset [2] because the latter does not fully work or >> maintained at the moment. >> >> Regarding the missing drivers note that the respective dependencies are >> selectively fetched by gradle when you run the inteTest* tasks, e.g.,: >> ./gradlew integTestPostgresql >> >> In order to run tests against Firebolt you will also have to touch a few >> places in the build script [3, 4, 5]. >> >> Best, >> Stamatis >> >> [1] >> >> https://github.com/apache/calcite/blob/a81cfb2ad001589929e190939cf4db928ebac386/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java#L163 >> [2] >> >> https://github.com/vlsi/calcite-test-dataset/blob/829ea54433dd9a4cf4d4a37da13c2dea66a24524/postgresql/pom.xml >> [3] >> >> https://github.com/apache/calcite/blob/a81cfb2ad001589929e190939cf4db928ebac386/core/build.gradle.kts#L43 >> [4] >> >> https://github.com/apache/calcite/blob/a81cfb2ad001589929e190939cf4db928ebac386/core/build.gradle.kts#L84 >> [5] >> >> https://github.com/apache/calcite/blob/a81cfb2ad001589929e190939cf4db928ebac386/core/build.gradle.kts#L266 >> >> On Mon, Apr 4, 2022 at 11:47 AM Raghav Sharma < >> [email protected]> >> wrote: >> >>> Thankyou for the update. >>> >>> We will file a JIRA case with the link of the PR raised for the Calcite >>> team to look into the code of the 'FireboltSqlDialect.java' file. >>> >>> Also, like it was mentioned before we did add a value 'FIREBOLT' to the >>> list of allowable values for the 'TEST_DB' inside >>> 'CalciteSystemProperty.java' along with an enum to 'DatabaseInstance' of >>> the same in 'CalciteAssert.java'. When we tried to run the command: >>> ‘./gradlew >>> -Dcalcite.test.db=FIREBOLT test’, it couldn't locate the driver. We also >>> checked with an existing enum i.e 'POSTGRESQL', the same issue persists. >> It >>> could not locate the driver for postgresql too. >>> >>> PFB the test report for FIREBOLT: >>> 7023 tests completed, 427 failed, 96 skipped >>> Most of these failed due to the driver issue. If this is broken, what >>> solution do you have? >>> >>> >>> Best Regards, >>> Raghav Sharma | SDE 1 | Data Engineering >>> M: +91 7087637086 I Mail: [email protected] >>> <https://www.sigmoid.com/> >>> >>> >>> On Sat, Apr 2, 2022 at 1:58 AM Julian Hyde <[email protected]> >> wrote: >>> >>>> Sorry I was slow to respond to your earlier message. >>>> >>>> (No need to add me to the thread - I am on the list, just backlogged. >> By >>>> the way, I am on vacation for the next ten days, so don’t expect >>> responses >>>> from me. Other project members may or may not be able to help.) >>>> >>>> Please log a JIRA case, as I asked. Then we can have discussions in >> that >>>> case. >>>> >>>> Re 1. I’m surprised that double-quote doesn’t work in >>>> RelToSqlConverterTest. Other dialects, such as Postgres, also use >>>> double-quote to quote identifiers. If you post a PR we could perhaps >> look >>>> at your code. >>>> >>>> Re 2. I think you will need to add “FIREBOLT” to the list of allowable >>>> values for the TEST_DB parameter [1] and also add a value to enum >>>> DatabaseInstance [2]. >>>> >>>> Julian >>>> >>>> [1] >>>> >>> >> https://github.com/apache/calcite/blob/a8a6569e6ba75efe9d5725c49338a7f181d3ab5c/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java#L166 >>>> < >>>> >>> >> https://github.com/apache/calcite/blob/a8a6569e6ba75efe9d5725c49338a7f181d3ab5c/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java#L166 >>>> >>>> >>>> >>>> [2] >>>> >>> >> https://github.com/apache/calcite/blob/bf56743554ea27d250d41db2eb83806f9e626b55/testkit/src/main/java/org/apache/calcite/test/CalciteAssert.java#L1904 >>>> < >>>> >>> >> https://github.com/apache/calcite/blob/bf56743554ea27d250d41db2eb83806f9e626b55/testkit/src/main/java/org/apache/calcite/test/CalciteAssert.java#L1904 >>>> >>>> >>>> >>>>> On Apr 1, 2022, at 5:07 AM, Raghav Sharma < >>> [email protected]> >>>> wrote: >>>>> >>>>> Adding Julian to the thread. >>>>> >>>>> >>>>> Thanks & Regards, >>>>> Raghav Sharma | SDE 1 | Data Engineering >>>>> M: +91 7087637086 I Mail: [email protected] >>>>> <https://www.sigmoid.com/> >>>>> >>>>> >>>>> On Thu, Mar 31, 2022 at 2:39 PM Raghav Sharma < >>>> [email protected]> >>>>> wrote: >>>>> >>>>>> Hey Julian! >>>>>> I followed your suggestions related to the dialect/plugin we want to >>>>>> contribute to Calcite for Firebolt. Some of the issues that we faced >>> so >>>> far: >>>>>> >>>>>> 1. Tests are failing in the RelToSqlConverterTest.java file for >>> Firebolt >>>>>> reason being the fact the actual parsed query does not contain the >>>>>> 'Identifier Quote String'. For Firebolt, we've chosen '\"' as the >>>>>> 'Identifier Quote String'. Have followed the approach that other >>>> dialects >>>>>> have used and therefore have added support for this in the dialect >>> file >>>> as >>>>>> well as in SqlDialect.java and SqlDialectFactoryImpl.java. Have even >>>>>> implemented the method 'withFirebolt()' in the test file. Please >> guide >>>> what >>>>>> else needs to be done for this to be fixed and our test cases to be >>>> passed. >>>>>> >>>>>> 2. After adding 'FIREBOLT' to "TEST_DB' in >> CalciteSystemProperty.java >>>> and >>>>>> running the command: ‘./gradlew -Dcalcite.test.db=FIREBOLT test’, >>>>>> Calcite throws an error that it could not find the enum for >> 'FIREBOLT' >>>> in >>>>>> CalciteAssert.java file. Even after adding the enum either with >>>> Firebolt's >>>>>> connection string or using one of the given ones, say POSTGRESQL, it >>>>>> couldn't locate the driver in both cases. Need your suggestion to >>> tackle >>>>>> this one. >>>>>> >>>>>> >>>>>> Thanks & Regards, >>>>>> Raghav Sharma | SDE 1 | Data Engineering >>>>>> M: +91 7087637086 I Mail: [email protected] >>>>>> <https://www.sigmoid.com/> >>>>>> >>>>>> >>>>>> On Tue, Mar 29, 2022 at 1:50 AM Raghav Sharma < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Thank you for the clarification Julian. I will send a mail to >>>>>>> [email protected] for subscribing. >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Tue, 29 Mar 2022 at 1:45 AM Julian Hyde <[email protected]> >> wrote: >>>>>>> >>>>>>>> You did receive a response[1]. But you didn't receive it because >> you >>>>>>>> are not subscribed to this list. Please subscribe[2]. >>>>>>>> >>>>>>>> Julian >>>>>>>> >>>>>>>> [1] >>> https://lists.apache.org/thread/39obrhwpd95bvhc9sb2n4z6zrrwbc21x >>>>>>>> >>>>>>>> [2] https://calcite.apache.org/community/#mailing-lists >>>>>>>> >>>>>>>> On Mon, Mar 28, 2022 at 1:11 PM Raghav Sharma >>>>>>>> <[email protected]> wrote: >>>>>>>>> >>>>>>>>> Gentle reminder! >>>>>>>>> We are expecting a response so that we can move ahead with this >>>>>>>> potential >>>>>>>>> contribution. >>>>>>>>> >>>>>>>>> >>>>>>>>> Thanks & Regards, >>>>>>>>> Raghav Sharma | SDE 1 | Data Engineering >>>>>>>>> M: +91 7087637086 I Mail: [email protected] >>>>>>>>> <https://www.sigmoid.com/> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, Mar 24, 2022 at 3:47 PM Raghav Sharma < >>>>>>>> [email protected]> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Greetings from Sigmoid and Firebolt! >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> This mail is in reference towards contributing a custom sql >>> dialect >>>>>>>> to >>>>>>>>>> Calcite. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> PFB some queries that we have regarding testing the dialect and >>>>>>>> further >>>>>>>>>> for contributing: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> 1. We have developed a SQL dialect for Firebolt. Need to test it >>>>>>>> against >>>>>>>>>> the database using calcite. How can we do so? Is this something >>> that >>>>>>>> can be >>>>>>>>>> pulled off using the JDBC driver(if so, how?) or is there >> another >>>>>>>> way? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> 2. There are some test files that contain tests for specific >>>>>>>> dialects. >>>>>>>>>> Before contributing, should we alter those for Firebolt(if >>> required) >>>>>>>> or is >>>>>>>>>> it something that will be taken care of by the Calcite team? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> 3. Is opening a JIRA case mandatory? Can we raise a PR directly >> to >>>>>>>> add >>>>>>>>>> Firebolt to the master branch of Calcite? Please guide us if >>> there’s >>>>>>>> an >>>>>>>>>> alternate option. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> We are willing to connect over a call as well if that works for >>> you. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Thanks & Regards, >>>>>>>>>> Raghav Sharma | SDE 1 | Data Engineering >>>>>>>>>> M: +91 7087637086 I Mail: [email protected] >>>>>>>>>> <https://www.sigmoid.com/> >>>>>>>>>> >>>>>>>>> -- >>>>>>>>> Thanks & Regards, >>>>>>>>> Raghav Sharma | SDE 1 | Data Engineering >>>>>>>>> M: +91 7087637086 I Mail: [email protected] >>>>>>>>> <https://www.sigmoid.com/> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> -- >>>>>>> Thanks & Regards, >>>>>>> Raghav Sharma | SDE 1 | Data Engineering >>>>>>> M: +91 7087637086 I Mail: [email protected] >>>>>>> <https://www.sigmoid.com/> >>>>>>> >>>>>> >>>>> >>>>> -- >>>>> >>>>> >>>> >>>> >>> >>> -- >>> >>> >>> >> > -- > Thanks & Regards, > Raghav Sharma | SDE 1 | Data Engineering > M: +91 7087637086 I Mail: [email protected] > <https://www.sigmoid.com/> > > -- > >
