Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Drill, mail # dev - Review Request 14027: Adding Limit operator end to end


Copy link to this message
-
Re: Review Request 14027: Adding Limit operator end to end
Jacques Nadeau 2013-10-10, 17:57
Can you start with my test that does order and limit in jdbctest?  Maybe
make a copy of that test and test yours.  We should make sure the logical
plan is correct before we validate the rest of the pipeline.
On Thu, Oct 10, 2013 at 10:52 AM, Timothy Chen <[EMAIL PROTECTED]> wrote:

> Hi Jacques,
>
> I just downloaded your commit and tried it locally, and the following query
> still doesn't work:
>
> SELECT _MAP['R_NAME'] from "sample-data/region.parquet" ORDER BY 1 DESC
> LIMIT 3;
>
> The Sort is now inserted and the results are ordered, but all 5 results are
> returned.
>
> I'll look into it more just wanted to let you know.
>
> Tim
>
>
> On Wed, Oct 9, 2013 at 11:14 AM, Jacques Nadeau <[EMAIL PROTECTED]>
> wrote:
>
> > Two main things, your rules weren't exactly right and sometimes the
> > ENUMERABLE_* rules would actually be chosen over your rules.  I removed
> > them since when we are doing transformation to logical plan, we don't
> want
> > to do any data operations.
> >
> >
> > On Wed, Oct 9, 2013 at 11:10 AM, Timothy Chen <[EMAIL PROTECTED]> wrote:
> >
> > > Amazing!
> > >
> > > Can you maybe explain a bit more why the sort is not getting
> populated? I
> > > see you removed the ENUMERABLE rules and added Drill.CONVENTION in the
> > > input.
> > >
> > > Tim
> > >
> > >
> > > On Wed, Oct 9, 2013 at 11:00 AM, Jacques Nadeau <[EMAIL PROTECTED]>
> > > wrote:
> > >
> > > > Hey Tim,
> > > >
> > > > I believe I fixed the problems.  You can get it from:
> > > >
> > > > https://github.com/jacques-n/incubator-drill/tree/limit_op
> > > >
> > > >
> > > > On Sun, Oct 6, 2013 at 1:09 PM, Timothy Chen <[EMAIL PROTECTED]>
> > wrote:
> > > >
> > > >> Hi Julian,
> > > >>
> > > >> Do you think you can take a look sometime?
> > > >>
> > > >> Tim
> > > >>
> > > >>
> > > >> On Sun, Sep 29, 2013 at 3:23 PM, Timothy Chen <[EMAIL PROTECTED]>
> > > wrote:
> > > >>
> > > >>> Btw, the code is at
> > > >>> https://github.com/tnachen/incubator-drill/tree/limit_op
> > > >>>
> > > >>> Tim
> > > >>>
> > > >>>
> > > >>> On Sun, Sep 29, 2013 at 3:22 PM, Timothy Chen <[EMAIL PROTECTED]>
> > > wrote:
> > > >>>
> > > >>>> I tried that as well earlier, but still the same result.
> > > >>>>
> > > >>>> It's creating the input and sorts, but in the end it won't call
> the
> > > >>>> DrillImplementor for Rels at all.
> > > >>>>
> > > >>>>  I also tried taking out the replace, but same results too.
> > > >>>>
> > > >>>> It wonder if the best route (cheapest) as able to skip the one
> > copied?
> > > >>>>
> > > >>>> I believe I followed mostly what you suggested that is follow
> > > >>>> EnumerableLimitRel impl.
> > > >>>>
> > > >>>> Tim
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>> On Sun, Sep 29, 2013 at 3:01 PM, Julian Hyde <
> [EMAIL PROTECTED]
> > > >wrote:
> > > >>>>
> > > >>>>>
> > > >>>>> On Sep 28, 2013, at 1:01 AM, Timothy Chen <[EMAIL PROTECTED]>
> > wrote:
> > > >>>>>
> > > >>>>> > Here is one of the trials I tried:
> > > >>>>> >
> > > >>>>> > -    final RelTraitSet traits > > > >>>>> sort.getTraitSet().plus(DrillRel.CONVENTION);
> > > >>>>> > +    final RelTraitSet traits = sort.getTraitSet();
> > > >>>>> >     RelNode input = sort.getChild();
> > > >>>>> >     if (!sort.getCollation().getFieldCollations().isEmpty()) {
> > > >>>>> >       input = sort.copy(
> > > >>>>> > -          sort.getTraitSet().replace(RelCollationImpl.EMPTY),
> > > >>>>> > -          input,
> > > >>>>> > -          RelCollationImpl.EMPTY,
> > > >>>>> > +          sort.getTraitSet(),
> > > >>>>> > +          sort,
> > > >>>>> > +          sort.getCollation(),
> > > >>>>> >           null,
> > > >>>>> >           null);
> > > >>>>> >     }
> > > >>>>>
> > > >>>>> The
> > > >>>>>
> > > >>>>>  +  sort,
> > > >>>>>
> > > >>>>> line should be
> > > >>>>>
> > > >>>>>  + input,
> > > >>>>>
> > > >>>>> otherwise you are creating a sort on top of a sort. Give that a
> > try.
> > > >>>>>
> > > >>>>> Julian
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>
> > > >>
> > > >
> >