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