trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zellerh <...@git.apache.org>
Subject [GitHub] trafodion pull request #1557: Changes for [TRAFODION-3065] and [TRAFODION-29...
Date Wed, 09 May 2018 19:22:47 GMT
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1557#discussion_r187144495
  
    --- Diff: core/sql/comexe/ComTdbFastTransport.h ---
    @@ -395,9 +399,10 @@ class ComTdbFastExtract : public ComTdb
       UInt16       filler_;                                      // 130 - 131
       UInt32       childDataRowLen_;                             // 132 - 135
       Int64        modTSforDir_;                                 // 136 - 143
    +  UInt16       hdfsIoByteArraySize_;                         // 144 - 147 
    --- End diff --
    
    Not that it really matters a lot, but I think this only takes up two bytes, 144-145?
    Another comment: This doesn't give a hint at all about what the unit is, bytes, KB, MB?
IMHO, it would be better to just use an int and specify the size in bytes, to avoid this problem.
If you want to keep the KB unit, I would recommend naming this hdfsIoByteArraySizeInKB_.
    This is a short (see comment about unsigned short below). Is 32 MB max IO byte array size
enough?


---

Mime
View raw message