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