Thanks for doing this Siddarth! It sounds like a good idea to post a WIP
so we can all take a look and try out some things out. There's a lot going
on so it's difficult to try to solve it all on paper, so maybe as we can
start with something very rough and take it from there. I think we all
agree on the requirements and goals, so that's a good start :)
Bryan
On Tue, Oct 3, 2017 at 3:58 PM, Siddharth Teotia <siddha...@dremio.com>
wrote:
> Anyway, I will create a PR in some time for the WIP prototype. I think once
> people eyeball the code there, we may have a consensus.
>
>
> On Tue, Oct 3, 2017 at 3:53 PM, Siddharth Teotia <siddha...@dremio.com>
> wrote:
>
> > Li,
> >
> > This is exactly what I was referring to in my previous email. I think if
> > we have the opportunity (which we have now), we should see if complex
> > template code like this can be removed unless it is absolutely necessary.
> > This is why I was suggesting if we can just not have any code generated
> > classes.
> >
> > Thanks,
> > Siddharth
> >
> > On Tue, Oct 3, 2017 at 3:03 PM, Li Jin <ice.xell...@gmail.com> wrote:
> >
> >> Siddharth,
> >>
> >> Thanks for the update. Without really sit down and do the prototype, my
> >> opinions can be wrong. But,
> >>
> >> I think a lot of the complication are code like this:
> >>
> >> <#elseif minor.class == "Decimal">
> >> public void get(int index, ${minor.class}Holder holder) {
> >> holder.start = index * ${type.width};
> >> holder.buffer = data;
> >> holder.scale = scale;
> >> holder.precision = precision;
> >> }
> >>
> >> The fact that we have to have a special block for Decimal is eyesore to
> >> me.
> >> And it's not really shared with any other classes.
> >>
> >> Things like these are not great but might be ok?
> >>
> >> <#if type.width == 4>
> >> public long getTwoAsLong(int index) {
> >> return data.getLong(index * ${type.width});
> >> }
> >>
> >> </#if>
> >>
> >> Sorry I couldn't provide too much useful feedback without digging into
> the
> >> template, but this is any general feeling about these templates - too
> many
> >> "if" to types like "Interval" "Decimal" "Timestamp"
> >>
> >>
> >>
> >> On Tue, Oct 3, 2017 at 3:59 PM, Siddharth Teotia <siddha...@dremio.com>
> >> wrote:
> >>
> >> > I am in the middle of a simple prototype that has the basic
> >> implementation
> >> > of BaseFixedWidthVector, FixedValueVectorsPrototype.java (template)
> to
> >> > generate a simple IntVector using the proposal mentioned in the
> >> document.
> >> >
> >> > I have realized that even though the LOCs in existing templates are
> >> reduced
> >> > by 30-40% since bunch of common/basic functionality is moved to super
> >> class
> >> > BaseFixedWidthVector, the major source of pain (giant and complex if
> >> > conditions) associated with code generation is in accessor and mutator
> >> > which is still part of templates.
> >> >
> >> > I am trying to err on the side of not using templates at all since I
> >> feel
> >> > there is not much of gain from this refactoring project if the code in
> >> > templates is still complex and requires regular addition/modification
> >> when
> >> > adding new types. We are probably better off writing multiple sub
> >> classes
> >> > (with duplicate code as applicable)
> >> >
> >> > Thoughts?
> >> >
> >> > I can create a PR from this prototype code once it in reasonable shape
> >> for
> >> > review but was wondering if people have any opinion.
> >> >
> >> > Thanks,
> >> > Sidd
> >> >
> >> > On Tue, Oct 3, 2017 at 3:16 AM, Siddharth Teotia <
> siddha...@dremio.com>
> >> > wrote:
> >> >
> >> > > Hi All,
> >> > >
> >> > > You should have received an invitation to edit the following
> document.
> >> > > Please feel free to add comments or additional content.
> >> > >
> >> > > https://docs.google.com/document/d/1rl0PK5OnbQAnFUrhd4bQPtP0u7930
> >> > > sBKKaiyggOY7t4/edit
> >> > >
> >> > > Let me know if the document is not editable.
> >> > >
> >> > > Thanks,
> >> > > Siddharth
> >> > >
> >> >
> >>
> >
> >
>
