Details

      Description

      Vectorized row batch object will represent the row batch that vectorized operators will work on. Refer to design spec attached to HIVE-4160 for details.

      1. HIVE-4284.3.patch
        26 kB
        Eric Hanson
      2. HIVE-4284.4.patch
        26 kB
        Eric Hanson
      3. HIVE-4284.5.patch
        27 kB
        Eric Hanson

        Issue Links

          Activity

          Hide
          ehans Eric Hanson added a comment -

          Adds VectorizedRowBatch class in org.apache.hadoop.hive.ql.exec.vector, plus unit test.

          Show
          ehans Eric Hanson added a comment - Adds VectorizedRowBatch class in org.apache.hadoop.hive.ql.exec.vector, plus unit test.
          Hide
          ehans Eric Hanson added a comment -

          This is to be applied to the "vectorization" branch off the hive main trunk.

          Show
          ehans Eric Hanson added a comment - This is to be applied to the "vectorization" branch off the hive main trunk.
          Hide
          jnp Jitendra Nath Pandey added a comment -

          A few comments on the patch.

          • Some files seem to have tab characters, the indentation is longer than two spaces.
          • Please limit the length of a line to 80 chars.
          • Javadocs are not correctly formatted in many places, and please make sure all public methods have a javadoc.
          • There are some test fields in BytesColumnVector, can that be avoided?
          • There is code to load random data into VectorizedRowBatch for testing. Since these are public classes, its better to provide test utilities to load random data instead of putting code in the classes themselves.
          Show
          jnp Jitendra Nath Pandey added a comment - A few comments on the patch. Some files seem to have tab characters, the indentation is longer than two spaces. Please limit the length of a line to 80 chars. Javadocs are not correctly formatted in many places, and please make sure all public methods have a javadoc. There are some test fields in BytesColumnVector, can that be avoided? There is code to load random data into VectorizedRowBatch for testing. Since these are public classes, its better to provide test utilities to load random data instead of putting code in the classes themselves.
          Hide
          namit Namit Jain added a comment -

          Comments from Jitendra.

          For a big patch like this, it would be very useful to have a phabricator entry

          Show
          namit Namit Jain added a comment - Comments from Jitendra. For a big patch like this, it would be very useful to have a phabricator entry
          Hide
          ehans Eric Hanson added a comment -

          new patch that addresses Jitendra's comments

          Show
          ehans Eric Hanson added a comment - new patch that addresses Jitendra's comments
          Hide
          cwsteinbach Carl Steinbach added a comment -

          Hive's coding conventions are described here:
          https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConvention

          Please review this and correct the formatting issues in this patch.

          Please limit the length of a line to 80 chars.

          100 characters is acceptable.

          Show
          cwsteinbach Carl Steinbach added a comment - Hive's coding conventions are described here: https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConvention Please review this and correct the formatting issues in this patch. Please limit the length of a line to 80 chars. 100 characters is acceptable.
          Hide
          ehans Eric Hanson added a comment -

          removed unwanted .rej file

          Show
          ehans Eric Hanson added a comment - removed unwanted .rej file
          Hide
          ehans Eric Hanson added a comment -
          Show
          ehans Eric Hanson added a comment - Code review link: https://reviews.facebook.net/differential/diff/32307/
          Hide
          ehans Eric Hanson added a comment -

          Disregard above code review link. Instead please use this:
          https://reviews.apache.org/r/10592/

          Show
          ehans Eric Hanson added a comment - Disregard above code review link. Instead please use this: https://reviews.apache.org/r/10592/
          Hide
          cwsteinbach Carl Steinbach added a comment -

          I left some comments on reviewboard.

          Show
          cwsteinbach Carl Steinbach added a comment - I left some comments on reviewboard.
          Hide
          ehans Eric Hanson added a comment -

          Updated based on code review comments.

          Show
          ehans Eric Hanson added a comment - Updated based on code review comments.
          Hide
          ehans Eric Hanson added a comment -

          Uploaded new diff to review tool at https://reviews.apache.org/r/10592/

          Show
          ehans Eric Hanson added a comment - Uploaded new diff to review tool at https://reviews.apache.org/r/10592/
          Hide
          ehans Eric Hanson added a comment -

          modified patch with updates based on code review comments

          Show
          ehans Eric Hanson added a comment - modified patch with updates based on code review comments
          Hide
          ehans Eric Hanson added a comment -

          New diff available for review at https://reviews.apache.org/r/10592/

          Show
          ehans Eric Hanson added a comment - New diff available for review at https://reviews.apache.org/r/10592/
          Hide
          cwsteinbach Carl Steinbach added a comment -

          +1

          Show
          cwsteinbach Carl Steinbach added a comment - +1
          Hide
          ashutoshc Ashutosh Chauhan added a comment -

          Committed to vectorization branch. Thanks, Eric!
          Thanks Jitendra and Carl for reviewing!

          Show
          ashutoshc Ashutosh Chauhan added a comment - Committed to vectorization branch. Thanks, Eric! Thanks Jitendra and Carl for reviewing!

            People

            • Assignee:
              ehans Eric Hanson
              Reporter:
              jnp Jitendra Nath Pandey
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development