You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by vdiravka <gi...@git.apache.org> on 2016/05/11 12:23:17 UTC

[GitHub] drill pull request: DRILL-2385: Count on complex objects failed wi...

GitHub user vdiravka opened a pull request:

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

    DRILL-2385: Count on complex objects failed with missing function implementation

    - added MapHolder, ListHolder;
    - added isNewTopLevelSchema() to OutputMutator interface;
    - added testCountComplexObjects() unit test.

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

    $ git pull https://github.com/vdiravka/drill DRILL-2385

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

    https://github.com/apache/drill/pull/501.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 #501
    
----
commit 8d2f49042187a943e2d820c61ac49e0cc6b794d2
Author: Vitalii Diravka <vi...@gmail.com>
Date:   2016-04-21T11:41:00Z

    DRILL-2385: Count on complex objects failed with missing function implementation
    - added MapHolder, ListHolder;
    - added isNewTopLevelSchema() to OutputMutator interface;
    - added testCountComplexObjects() unit test.

----


---
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: DRILL-2385: Count on complex objects failed with missing...

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

    https://github.com/apache/drill/pull/501#discussion_r65275657
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ---
    @@ -196,8 +196,8 @@ public IterOutcome next() {
                 releaseAssets();
                 done = true;  // have any future call to next() return NONE
     
    -            if (mutator.isNewSchema()) {
    -              // This last reader has a new schema (e.g., we have a zero-row
    +            if (mutator.isNewTopLevelSchema()) {
    --- End diff --
    
    I don't see why this condition is better than isNewSchema. Looks to me that any time this is true, isNewSchema will also be true. Why don't we want to check the condition that the deeperSchema might have changed?


---
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 #501: DRILL-2385: Count on complex objects failed with missing f...

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

    https://github.com/apache/drill/pull/501
  
    Changes merged into master with commit id f86c4fa


---
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 #501: DRILL-2385: Count on complex objects failed with mi...

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

    https://github.com/apache/drill/pull/501#discussion_r66117639
  
    --- Diff: exec/java-exec/src/test/resources/complex/json/complex.json ---
    @@ -0,0 +1,30 @@
    +{"sia":[1, 11, 101, 1001],
    + "sfa":[0.0, 1.01, 10.222, 10.0006789],
    + "soa":[{"in":1},{"in":1,"fl":1.12345}, {"in":1, "fl":10.12345, "nul":null}, {"in":1, "fl":10.6789, "nul":null, "bool":true, "str":"here is a string at row 1"}],
    + "oooi":{"oa":{"oab":{"oabc":1}}},
    + "odd": [
    +    [[1],[],[3]],
    +    [],
    +    [[5]]
    + ]
    +}
    +{"sia":[2, 12, 102, 1002],
    + "sfa":[0.0, 2.01, 20.222, 20.0006789],
    + "soa":[{"in":2},{"in":2,"fl":2.12345}, {"in":2, "fl":20.12345, "nul":"not null"}, {"in":2, "fl":20.6789, "nul":null, "bool":false, "str":"here is a string at row 2"}],
    + "oooi":{"oa":{"oab":{"oabc":2}}},
    + "odd": [
    +    [[1],[],[3]],
    +    [],
    +    [[5]]
    + ]
    +}
    +{"sia":[3, 13, 103, 1003],
    + "sfa":[0.0, 3.01, 30.222, 30.0006789],
    + "soa":[{"in":3},{"in":3,"fl":3.12345}, {"in":3, "fl":30.12345, "nul":"not null"}, {"in":3, "fl":30.6789, "nul":"not null", "bool":true, "str":"here is a string at row 3"}],
    + "oooi":{"oa":{"oab":{"oabc":3}}},
    + "odd": [
    +    [[1],[],[3]],
    +    [],
    +    [[5]]
    + ]
    +}
    --- End diff --
    
    added


---
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 #501: DRILL-2385: Count on complex objects failed with mi...

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

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


---
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: DRILL-2385: Count on complex objects failed with missing...

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

    https://github.com/apache/drill/pull/501#discussion_r65275697
  
    --- Diff: exec/java-exec/src/test/resources/complex/json/complex.json ---
    @@ -0,0 +1,30 @@
    +{"sia":[1, 11, 101, 1001],
    + "sfa":[0.0, 1.01, 10.222, 10.0006789],
    + "soa":[{"in":1},{"in":1,"fl":1.12345}, {"in":1, "fl":10.12345, "nul":null}, {"in":1, "fl":10.6789, "nul":null, "bool":true, "str":"here is a string at row 1"}],
    + "oooi":{"oa":{"oab":{"oabc":1}}},
    + "odd": [
    +    [[1],[],[3]],
    +    [],
    +    [[5]]
    + ]
    +}
    +{"sia":[2, 12, 102, 1002],
    + "sfa":[0.0, 2.01, 20.222, 20.0006789],
    + "soa":[{"in":2},{"in":2,"fl":2.12345}, {"in":2, "fl":20.12345, "nul":"not null"}, {"in":2, "fl":20.6789, "nul":null, "bool":false, "str":"here is a string at row 2"}],
    + "oooi":{"oa":{"oab":{"oabc":2}}},
    + "odd": [
    +    [[1],[],[3]],
    +    [],
    +    [[5]]
    + ]
    +}
    +{"sia":[3, 13, 103, 1003],
    + "sfa":[0.0, 3.01, 30.222, 30.0006789],
    + "soa":[{"in":3},{"in":3,"fl":3.12345}, {"in":3, "fl":30.12345, "nul":"not null"}, {"in":3, "fl":30.6789, "nul":"not null", "bool":true, "str":"here is a string at row 3"}],
    + "oooi":{"oa":{"oab":{"oabc":3}}},
    + "odd": [
    +    [[1],[],[3]],
    +    [],
    +    [[5]]
    + ]
    +}
    --- End diff --
    
    Missing a newline at end of file.


---
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: DRILL-2385: Count on complex objects failed with missing...

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

    https://github.com/apache/drill/pull/501#discussion_r65284923
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java ---
    @@ -78,6 +78,9 @@
         precedenceMap.put(MinorType.INTERVALYEAR, i+= 2);
         precedenceMap.put(MinorType.INTERVAL, i+= 2);
         precedenceMap.put(MinorType.UNION, i += 2);
    +    precedenceMap.put(MinorType.MAP, i += 2);
    --- End diff --
    
    List and Map cannot have a higher precedence than Map. Also, it is not clear what the precedence of List and Map should be.  Can you post a specific question to the dev list? 


---
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 #501: DRILL-2385: Count on complex objects failed with mi...

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

    https://github.com/apache/drill/pull/501#discussion_r66120908
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ---
    @@ -196,8 +196,8 @@ public IterOutcome next() {
                 releaseAssets();
                 done = true;  // have any future call to next() return NONE
     
    -            if (mutator.isNewSchema()) {
    -              // This last reader has a new schema (e.g., we have a zero-row
    +            if (mutator.isNewTopLevelSchema()) {
    --- End diff --
    
    This method was added for the purpose to make count on map even when deeper schema (child fields of map) was changed. But now I  noticed that even simple select query on Map with different deeper (child) schema doesn't work. That's why no need in this method.
    I returned to isNewShema method.


---
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 #501: DRILL-2385: Count on complex objects failed with missing f...

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

    https://github.com/apache/drill/pull/501
  
    +1 LGTM


---
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 #501: DRILL-2385: Count on complex objects failed with missing f...

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

    https://github.com/apache/drill/pull/501
  
    Squashing and rebasing to the master were done.


---
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 #501: DRILL-2385: Count on complex objects failed with mi...

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

    https://github.com/apache/drill/pull/501#discussion_r66118486
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/ResolverTypePrecedence.java ---
    @@ -78,6 +78,9 @@
         precedenceMap.put(MinorType.INTERVALYEAR, i+= 2);
         precedenceMap.put(MinorType.INTERVAL, i+= 2);
         precedenceMap.put(MinorType.UNION, i += 2);
    +    precedenceMap.put(MinorType.MAP, i += 2);
    --- End diff --
    
    It seems you meant "than Union".
    Devs agreed with the place in the end of precedenceMap just before Union
    Updated.


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