You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by paul-rogers <gi...@git.apache.org> on 2017/08/09 22:48:24 UTC

[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

GitHub user paul-rogers opened a pull request:

    https://github.com/apache/drill/pull/901

    DRILL-5709: Provide a value vector method to convert a vector to nullable

    Please see the DRILL-5709 for an explanation and example.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/paul-rogers/drill DRILL-5709

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/901.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #901
    
----
commit ced37d52d161daae333d6048150f92c181defa53
Author: Paul Rogers <pr...@maprtech.com>
Date:   2017-08-09T03:04:24Z

    DRILL-5709: Provide a value vector method to convert a vector to nullable
    
    Please see the DRILL-5709 for an explanation and example.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/901#discussion_r132824835
  
    --- Diff: exec/vector/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---
    @@ -133,5 +134,21 @@ public static boolean checkBufRefs(final ValueVector vv) {
       public BufferAllocator getAllocator() {
         return allocator;
       }
    +
    +  public static void fillBitsVector(UInt1Vector bits, int valueCount) {
    +
    +    // Create a new bits vector, all values non-null
    +
    +    bits.allocateNew(valueCount);
    +    UInt1Vector.Mutator bitsMutator = bits.getMutator();
    +    for (int i = 0; i < valueCount; i++) {
    +      bitsMutator.set(i, 1);
    +    }
    +  }
    --- End diff --
    
    Fixed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

Posted by Ben-Zvi <gi...@git.apache.org>.
Github user Ben-Zvi commented on a diff in the pull request:

    https://github.com/apache/drill/pull/901#discussion_r132806738
  
    --- Diff: exec/vector/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---
    @@ -133,5 +134,21 @@ public static boolean checkBufRefs(final ValueVector vv) {
       public BufferAllocator getAllocator() {
         return allocator;
       }
    +
    +  public static void fillBitsVector(UInt1Vector bits, int valueCount) {
    +
    +    // Create a new bits vector, all values non-null
    +
    +    bits.allocateNew(valueCount);
    +    UInt1Vector.Mutator bitsMutator = bits.getMutator();
    +    for (int i = 0; i < valueCount; i++) {
    +      bitsMutator.set(i, 1);
    +    }
    +  }
    --- End diff --
    
    And need to add:
    <code>
    bitsMutator.setValueCount(valueCount);
    </code>



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #901: DRILL-5709: Provide a value vector method to convert a vec...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the issue:

    https://github.com/apache/drill/pull/901
  
    @arina-ielchiieva, conflicts are resolved.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

Posted by Ben-Zvi <gi...@git.apache.org>.
Github user Ben-Zvi commented on a diff in the pull request:

    https://github.com/apache/drill/pull/901#discussion_r132806694
  
    --- Diff: exec/vector/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---
    @@ -133,5 +134,21 @@ public static boolean checkBufRefs(final ValueVector vv) {
       public BufferAllocator getAllocator() {
         return allocator;
       }
    +
    +  public static void fillBitsVector(UInt1Vector bits, int valueCount) {
    +
    +    // Create a new bits vector, all values non-null
    +
    +    bits.allocateNew(valueCount);
    +    UInt1Vector.Mutator bitsMutator = bits.getMutator();
    +    for (int i = 0; i < valueCount; i++) {
    +      bitsMutator.set(i, 1);
    +    }
    --- End diff --
    
    This loop may be a bit expensive; Is there a way to mark the whole bitmap with '1's "at once" ?  A la "memset()".  
    For example, keep a constant bitmap (of max length - 64K) someplace, and here just dup/copy it whole.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/drill/pull/901


---

[GitHub] drill pull request #901: DRILL-5709: Provide a value vector method to conver...

Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/901#discussion_r132824832
  
    --- Diff: exec/vector/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---
    @@ -133,5 +134,21 @@ public static boolean checkBufRefs(final ValueVector vv) {
       public BufferAllocator getAllocator() {
         return allocator;
       }
    +
    +  public static void fillBitsVector(UInt1Vector bits, int valueCount) {
    +
    +    // Create a new bits vector, all values non-null
    +
    +    bits.allocateNew(valueCount);
    +    UInt1Vector.Mutator bitsMutator = bits.getMutator();
    +    for (int i = 0; i < valueCount; i++) {
    +      bitsMutator.set(i, 1);
    +    }
    --- End diff --
    
    Sadly, Netty's `PlatformDependent` provides no equivalent of `memset`. Some tricks we could do, if tests show we have a performance issue:
    
    * Use `setLong()` to write the value 0x01010101010101L every 8 values, with bytes for the last < 8 values.
    * Create a heap buffer of some length n, filled with 0x01, and do a copy from that buffer to the direct memory, as many times as needed.
    
    Since both of these will make the code much more complex, let's measure to see the cost of the simple solution before we try them.
    
    Also, at a higher level, we should not even need this trick. A properly-designed value vector hierarchy would treat a non-nullable vector as a nullable vector that always returns `false` for `isNull()`. This is what the new readers do and it vastly simplifies the code.
    
    Sometimes it helps to go back to basics and remember Abstractions 101: well-designed abstractions are our friend.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill issue #901: DRILL-5709: Provide a value vector method to convert a vec...

Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the issue:

    https://github.com/apache/drill/pull/901
  
    +1 


---

[GitHub] drill issue #901: DRILL-5709: Provide a value vector method to convert a vec...

Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:

    https://github.com/apache/drill/pull/901
  
    @paul-rogers please resolve the conflicts.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---