You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Julian Hyde <ju...@gmail.com> on 2013/05/07 07:06:08 UTC

Another pair of eyes...

Hi Drillers,

I am having problems integrating the logical plan reference interpreter with the Optiq plans that I generate to implement SQL. Specifically with which "wrapper" elements I should expect to wrap the rows that come out of the reference interpreter.

I'd appreciate if another Drill developer could run my code in a debugger and see whether I am generating the wrong logical plan, or interpreting its results wrongly, or whether the reference interpreter is broken. In other words, I'd like to borrow another pair of eyes. Any volunteers?

Download the "optiq" branch from my github fork:

$ git clone git@github.com:julianhyde/incubator-drill.git
$ cd incubator-drill
$ git checkout optiq

Then run the "testCount" method of sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java. It ought to return something like this:

C=1; DEPTID=null
C=2; DEPTID=34
C=2; DEPTID=33
C=1; DEPTID=31

but actually returns this:

C=1; DEPTID={DEPTID=null, LASTNAME=John}
C=1; DEPTID={DEPTID=34, LASTNAME=Robinson}
C=1; DEPTID={DEPTID=33, LASTNAME=Steinburg}
C=1; DEPTID={DEPTID=31, LASTNAME=Rafferty}
C=1; DEPTID={DEPTID=33, LASTNAME=Jones}
C=1; DEPTID={DEPTID=34, LASTNAME=Smith}

As you can see, the DEPTID column has an extra level of nesting. I print the logical plan to stdout, so see whether that makes sense.

Julian

Re: Another pair of eyes...

Posted by Lisen Mu <im...@gmail.com>.
Julian,

glad you find the work useful.



On Mon, May 20, 2013 at 11:20 PM, Julian Hyde <ju...@gmail.com> wrote:

> On May 8, 2013, at 9:31 PM, Lisen Mu <im...@gmail.com> wrote:
>
> > I'm not familiar with the optiq inside but seems it has something to do
> with your last projection's ref. What's the purpose of  getHolder()?
> >
> > Maybe not the right thing to do, but the attached patch could produce
> output without nesting(but throws exceptions in some test...).
>
> Lisen,
>
> Your observations & patch helped get me through my mental block. I
> realized that "ref" is necessary for the "scan" operator, but not others.
> Therefore I made sure that each operator's JSON output the logical row type
> of the corresponding Optiq relational operator. Scan outputs records with a
> single field called _MAP; project contains whatever field(s) they asked
> for; filter outputs the same row type as its input; and so forth.
>
> I was able to obsolete the "holder" notion, and some other nasty duck tape
> [
> https://github.com/julianhyde/incubator-drill/commit/375dbf03157924860bfc6b3c7e1084cd91faa61d]. I also implemented aggregation over the weekend [
> https://github.com/julianhyde/incubator-drill/commit/febcb702c224f8aa148377272eb88929cfa15ee7].
>
> Thanks for your help.
>
> Julian

Re: Another pair of eyes...

Posted by Julian Hyde <ju...@gmail.com>.
On May 8, 2013, at 9:31 PM, Lisen Mu <im...@gmail.com> wrote:

> I'm not familiar with the optiq inside but seems it has something to do with your last projection's ref. What's the purpose of  getHolder()?
> 
> Maybe not the right thing to do, but the attached patch could produce output without nesting(but throws exceptions in some test...).

Lisen,

Your observations & patch helped get me through my mental block. I realized that "ref" is necessary for the "scan" operator, but not others. Therefore I made sure that each operator's JSON output the logical row type of the corresponding Optiq relational operator. Scan outputs records with a single field called _MAP; project contains whatever field(s) they asked for; filter outputs the same row type as its input; and so forth.

I was able to obsolete the "holder" notion, and some other nasty duck tape [ https://github.com/julianhyde/incubator-drill/commit/375dbf03157924860bfc6b3c7e1084cd91faa61d ]. I also implemented aggregation over the weekend [ https://github.com/julianhyde/incubator-drill/commit/febcb702c224f8aa148377272eb88929cfa15ee7 ].

Thanks for your help.

Julian

Re: Another pair of eyes...

Posted by Lisen Mu <im...@gmail.com>.
Julian,

I'm not familiar with the optiq inside but seems it has something to do
with your last projection's ref. What's the purpose of  getHolder()?

Maybe not the right thing to do, but the attached patch could produce
output without nesting(but throws exceptions in some test...).




On Tue, May 7, 2013 at 1:06 PM, Julian Hyde <ju...@gmail.com> wrote:

> Hi Drillers,
>
> I am having problems integrating the logical plan reference interpreter
> with the Optiq plans that I generate to implement SQL. Specifically with
> which "wrapper" elements I should expect to wrap the rows that come out of
> the reference interpreter.
>
> I'd appreciate if another Drill developer could run my code in a debugger
> and see whether I am generating the wrong logical plan, or interpreting its
> results wrongly, or whether the reference interpreter is broken. In other
> words, I'd like to borrow another pair of eyes. Any volunteers?
>
> Download the "optiq" branch from my github fork:
>
> $ git clone git@github.com:julianhyde/incubator-drill.git
> $ cd incubator-drill
> $ git checkout optiq
>
> Then run the "testCount" method of
> sandbox/prototype/sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java.
> It ought to return something like this:
>
> C=1; DEPTID=null
> C=2; DEPTID=34
> C=2; DEPTID=33
> C=1; DEPTID=31
>
> but actually returns this:
>
> C=1; DEPTID={DEPTID=null, LASTNAME=John}
> C=1; DEPTID={DEPTID=34, LASTNAME=Robinson}
> C=1; DEPTID={DEPTID=33, LASTNAME=Steinburg}
> C=1; DEPTID={DEPTID=31, LASTNAME=Rafferty}
> C=1; DEPTID={DEPTID=33, LASTNAME=Jones}
> C=1; DEPTID={DEPTID=34, LASTNAME=Smith}
>
> As you can see, the DEPTID column has an extra level of nesting. I print
> the logical plan to stdout, so see whether that makes sense.
>
> Julian