drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From daveoshinsky <...@git.apache.org>
Subject [GitHub] drill pull request #570: DRILL-4834 decimal implementation is vulnerable to ...
Date Wed, 10 Jan 2018 20:31:24 GMT
Github user daveoshinsky commented on a diff in the pull request:

    --- Diff: exec/vector/src/main/codegen/templates/ComplexWriters.java ---
    @@ -99,7 +99,7 @@ public void write(Nullable${minor.class?cap_first}Holder h) {
       <#if !(minor.class == "Decimal9" || minor.class == "Decimal18" || minor.class ==
"Decimal28Sparse" || minor.class == "Decimal38Sparse" || minor.class == "Decimal28Dense" ||
minor.class == "Decimal38Dense")>
       public void write${minor.class}(<#list fields as field>${field.type} ${field.name}<#if
field_has_next>, </#if></#list>) {
    -    mutator.addSafe(idx(), <#list fields as field>${field.name}<#if field_has_next>,
    +    mutator.addSafe(idx(), <#list fields as field><#if field.name == "scale"><#break></#if>${field.name}<#if
field_has_next && fields[field_index+1].name != "scale" >, </#if></#list>);
    --- End diff --
    I had a hard time understanding how this codegen stuff works, so some of the code I wrote
was due to my struggling to  understand how it is SUPPOSED to work.  As such, I found that
it needed to "break" out of code generation just after the "scale" argument, to avoid compilation
failure of the generated code (IIRC, it was quite a while ago), and that is why I coded it
this way.  What exact change are you suggesting?  To AND the field.name check=="scale" check
with a check that minor.class=="VarDecimal"?


View raw message