Thanks Julian! It would really help us in moving forward. On Tue, 12 Apr 2022 at 12:01 AM Julian Hyde <[email protected]> wrote:
> 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/> > > > > -- > > > > > > -- Thanks & Regards, Raghav Sharma | SDE 1 | Data Engineering M: +91 7087637086 I Mail: [email protected] <https://www.sigmoid.com/> --
