Regarding https://issues.apache.org/jira/browse/CALCITE-1150, "Create the a new DynamicRecordType, avoiding star expansion when working with this type”. This feature will be useful, and as I have said to Jacques would fit well within Calcite, but it’s a bit shapeless to me right now. I would like to see some validator tests (any maybe one or two sql-to-rel converter tests) so I get a feel for how it would work. When you have some tests can you post them in the JIRA case? I don’t think you should charge ahead with the implementation because I might not agree with the specification (i.e. the test cases).
I’l make the same comment in the JIRA case, and let’s continue the discussion there. Julian > On Apr 19, 2016, at 9:51 AM, Jinfeng Ni <[email protected]> wrote: > > @Jacques, > > Sorry for the delay. Let me spend couple of days this week to get > CALCITE-1150 back on track. Initially, I encountered one conflicting > change in Calcite master, and broke couple of unit tests. If I can not > get them solved, I'll ping you or Minji for a discussion. > > > > On Mon, Apr 18, 2016 at 5:31 PM, Jacques Nadeau <[email protected]> wrote: >> Hey All, >> >> Following up to get a status update. We made some good initial progress but >> it seems like people may have hit some challenges (or distractions). Can >> everyone report on how they are doing? >> >> Jinfeng, how are tests for CALCITE-1150 going? Can Minji help get together >> test cases for CALCITE-1150? Maybe you could provide guidance on the set of >> queries to test? >> >> thanks, >> Jacques >> >> >> -- >> Jacques Nadeau >> CTO and Co-Founder, Dremio >> >> On Thu, Mar 31, 2016 at 4:19 PM, Julian Hyde <[email protected]> wrote: >> >>> I’ve closed 1149, if we don’t need the feature. >>> >>> Yes, we need a unit test for 1151. I offered a suggestion how. >>> >>>> On Mar 31, 2016, at 11:59 AM, Sudheesh Katkam <[email protected]> >>> wrote: >>>> >>>> I submitted a patch for CALCITE-1151 < >>> https://issues.apache.org/jira/browse/CALCITE-1151> (with changes to >>> resolve a checkstyle error). I am waiting for comments regarding the unit >>> test. >>>> >>>> I added a comment to CALCITE-1149 < >>> https://issues.apache.org/jira/browse/CALCITE-1149> with the workaround >>> being used. >>>> >>>> Thank you, >>>> Sudheesh >>>> >>>>> On Mar 16, 2016, at 5:19 PM, Jacques Nadeau <[email protected]> wrote: >>>>> >>>>> Yes, I'm trying to work through the failing unit tests. >>>>> >>>>> I merged your change. >>>>> >>>>> In the future you can pick compare & create pull request on your branch >>> and >>>>> then change the target repo from apache to mine. >>>>> >>>>> thanks, >>>>> Jacques >>>>> >>>>> >>>>> -- >>>>> Jacques Nadeau >>>>> CTO and Co-Founder, Dremio >>>>> >>>>> On Wed, Mar 16, 2016 at 4:39 PM, Aman Sinha <[email protected]> >>> wrote: >>>>> >>>>>> Jacques, I wasn't sure how to create a pull request against your >>> branch; >>>>>> for CALCITE-1108 you can cherry-pick from here: >>>>>> >>>>>> >>> https://github.com/amansinha100/incubator-calcite/commits/calcite-drill-2 >>>>>> >>>>>> BTW, there are unit test failures on your branch which I assume is >>>>>> expected for now ? >>>>>> >>>>>> On Tue, Mar 15, 2016 at 6:56 PM, Jacques Nadeau <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Why don't you guys propose patches for my branch and I'll incorporate >>>>>> until >>>>>>> we get to a good state. Once we feel good about it, I'll clean up the >>>>>>> revision history. >>>>>>> >>>>>>> -- >>>>>>> Jacques Nadeau >>>>>>> CTO and Co-Founder, Dremio >>>>>>> >>>>>>> On Tue, Mar 15, 2016 at 11:01 AM, Jinfeng Ni <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> I'll add test for CALCITE-1150. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Tue, Mar 15, 2016 at 9:45 AM, Sudheesh Katkam < >>> [email protected] >>>>>>> >>>>>>>> wrote: >>>>>>>>> CALCITE-1149 [Extend CALCITE-845] < >>>>>>>> >>>>>>> >>>>>> >>> https://github.com/mapr/incubator-calcite/commit/bd73728a8297e15331ae956096eab0e15bbbbb3f >>>>>>>> >>>>>>>> does not need to be committed into Calcite. DRILL-4372 < >>>>>>>> https://issues.apache.org/jira/browse/DRILL-4372> supersedes that >>>>>> patch. >>>>>>>>> >>>>>>>>> I will add a test case for CALCITE-1151. >>>>>>>>> >>>>>>>>> Thank you, >>>>>>>>> Sudheesh >>>>>>>>> >>>>>>>>>> On Mar 15, 2016, at 9:04 AM, Aman Sinha <[email protected]> >>>>>> wrote: >>>>>>>>>> >>>>>>>>>> I'll add a test for CALCITE-1108. For 1105 I am not yet sure but >>>>>>> will >>>>>>>>>> look through the old drill commits to see what test was added >>> there. >>>>>>>>>> >>>>>>>>>> On Sun, Mar 13, 2016 at 11:15 PM, Minji Kim <[email protected]> >>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> I will add more test cases to CALCITE-1148 in addition to the ones >>>>>>>> already >>>>>>>>>>> there. I noticed a few more problems while testing the patch >>>>>> against >>>>>>>> drill >>>>>>>>>>> master. I am still working through these issues, so I will add >>>>>> more >>>>>>>> test >>>>>>>>>>> cases as I find/fix them. -Minji >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 3/13/16 10:54 PM, Jacques Nadeau wrote: >>>>>>>>>>> >>>>>>>>>>>> Hey All, >>>>>>>>>>>> >>>>>>>>>>>> I've been working on rebasing and tracking all the necessary >>>>>> commits >>>>>>>> that >>>>>>>>>>>> are on the Drill Calcite fork so that we can get back onto >>> master. >>>>>>> The >>>>>>>>>>>> current working branch is here: [1]. It includes the following >>>>>>> commits >>>>>>>>>>>> >>>>>>>>>>>> [CALCITE-1148] Fix RelTrait conversion (e.g. distribution, >>>>>>> collation), >>>>>>>>>>>> added test cases. (Minji Kim) #77def4a >>>>>>>>>>>> [CALCITE-991] Create separate FunctionCategories for table >>>>>> functions >>>>>>>> and >>>>>>>>>>>> macros (Julien Le Dem) #b1c203d >>>>>>>>>>>> [CALCITE-1149] Derive AVG’s return type by a customizable policy >>>>>>>> (Sudheesh >>>>>>>>>>>> Katkam) #18882cd >>>>>>>>>>>> [CALCITE-1151] Overriding the SqlSpecialOperator#createCall >>> method >>>>>>>> given >>>>>>>>>>>> the usage by CompoundIdentifierConverter (Sudheesh Katkam) >>>>>> #2320c7f >>>>>>>>>>>> [CALCITE-1108] Don't use 'SumEmptyIsZero' (SUM0) window aggregate >>>>>>>> until >>>>>>>>>>>> CALCITE-777 is fixed. (Aman Sinha) #13466fa >>>>>>>>>>>> [CALCITE-1107] Make SqlSumEmptyIsZeroAggFunction constructor >>>>>> public. >>>>>>>>>>>> (Jinfeng Ni) #b6c3178 >>>>>>>>>>>> [CALCITE-1106] Expose Constructor for ProjectJoinTransposeRule. >>>>>>> (Aman >>>>>>>>>>>> Sinha) #d169c37 >>>>>>>>>>>> [CALCITE-1105] Add return type-inference strategy for arithmetic >>>>>>>> operators >>>>>>>>>>>> when one of the arguments is ANY type. (Aman Sinha) #df818c9 >>>>>>>>>>>> [CALCITE-1150] Add DynamicRecordType and the concept of >>> unresolved >>>>>>>> star >>>>>>>>>>>> (Jinfeng Ni) #29c7771 >>>>>>>>>>>> [CALCITE-1152] Small ANY type fixes (Mehant Baid) #31efdda >>>>>>>>>>>> [CALCITE-528] Ensure uniquification is done in a case aware way >>>>>>>> according >>>>>>>>>>>> to type system and catalog policies. (Jacques Nadeau) #5a3d854 >>>>>>>>>>>> >>>>>>>>>>>> Many commits, listed below, don't have tests right now so I'd >>> like >>>>>>> to >>>>>>>> get >>>>>>>>>>>> people to raise their hand and work on tests for each of the >>>>>>> commits. >>>>>>>>>>>> >>>>>>>>>>>> [CALCITE-991] Create separate FunctionCategories for table >>>>>> functions >>>>>>>> and >>>>>>>>>>>> macros (Julien Le Dem) #b1c203d >>>>>>>>>>>> [CALCITE-1149] Derive AVG’s return type by a customizable policy >>>>>>>> (Sudheesh >>>>>>>>>>>> Katkam) #18882cd >>>>>>>>>>>> [CALCITE-1151] Overriding the SqlSpecialOperator#createCall >>> method >>>>>>>> given >>>>>>>>>>>> the usage by CompoundIdentifierConverter (Sudheesh Katkam) >>>>>> #2320c7f >>>>>>>>>>>> [CALCITE-1108] Don't use 'SumEmptyIsZero' (SUM0) window aggregate >>>>>>>> until >>>>>>>>>>>> CALCITE-777 is fixed. (Aman Sinha) #13466fa >>>>>>>>>>>> [CALCITE-1105] Add return type-inference strategy for arithmetic >>>>>>>> operators >>>>>>>>>>>> when one of the arguments is ANY type. (Aman Sinha) #df818c9 >>>>>>>>>>>> [CALCITE-1150] Add DynamicRecordType and the concept of >>> unresolved >>>>>>>> star >>>>>>>>>>>> (Jinfeng Ni) #29c7771 >>>>>>>>>>>> [CALCITE-1152] Small ANY type fixes (Mehant Baid) #31efdda >>>>>>>>>>>> [CALCITE-528] Ensure uniquification is done in a case aware way >>>>>>>> according >>>>>>>>>>>> to type system and catalog policies. (Jacques Nadeau) #5a3d854 >>>>>>>>>>>> >>>>>>>>>>>> Also note that there are currently 15 tests failing in this >>>>>> Calcite >>>>>>>> branch >>>>>>>>>>>> that I haven't yet tracked down. >>>>>>>>>>>> >>>>>>>>>>>> org.apache.calcite.test.SqlToRelConverterTest (10 tests) >>>>>>>>>>>> org.apache.calcite.test.JdbcTest (2 tests) >>>>>>>>>>>> org.apache.calcite.test.RelOptRulesTest.txt (1 test) >>>>>>>>>>>> org.apache.calcite.test.SqlValidatorTest.txt (1 test) >>>>>>>>>>>> org.apache.calcite.rel.rel2sql.RelToSqlConverterTest (1 test) >>>>>>>>>>>> >>>>>>>>>>>> Note that I also reworked the Schema changes items so that they >>>>>>> don't >>>>>>>> have >>>>>>>>>>>> any impact on code paths unless the system returns a >>>>>>>> DynamicRecordType. >>>>>>>>>>>> Once we get these changes looking good, we can move to making >>>>>> small >>>>>>>>>>>> modifications in the Drill codebase to use this new record type. >>>>>>>>>>>> >>>>>>>>>>>> Can people raise their hands to confirm they will be able to >>> write >>>>>>>> tests >>>>>>>>>>>> cases for issues they own? >>>>>>>>>>>> >>>>>>>>>>>> thanks, >>>>>>>>>>>> Jacques >>>>>>>>>>>> >>>>>>>>>>>> [1] >>>>>>>> https://github.com/jacques-n/incubator-calcite/tree/calcite-drill-2 >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> Jacques Nadeau >>>>>>>>>>>> CTO and Co-Founder, Dremio >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>> >>> >>>
