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

Reply via email to