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

Reply via email to