Github user cwestin closed the pull request at:
https://github.com/apache/drill/pull/181
---
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 enab
Github user cwestin commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145645022
This was merged.
---
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 featur
Github user jaltekruse commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145160952
Go for it
---
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
en
Github user parthchandra commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145160342
Just completed a build and unit test run successfully. I can merging this.
---
If your project is set up for it, you can reply to this email and have your
reply appe
Github user jaltekruse commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145158691
I'm running the unit tests on the changeset rebased on top of master, will
be merging shortly if it passes.
---
If your project is set up for it, you can reply to thi
Github user cwestin commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145149090
I just squashed and pushed with the valueCount + 1 for the
BaseRepeatedValueVector, along with using valueCount in getBufferSizeFor()
instead of getAccessor().getValueCou
Github user jaltekruse commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145117861
I'm okay with merging as is, I think it is worth keeping a JIRA open for
this refactoring. The fact that these behave inconsistently pretty much
guarantees that one of
Github user parthchandra commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145067195
Re OUTPUT_MEMORY_LIMIT: I'm fine if you leave it the way it is.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitH
Github user cwestin commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-145033096
I tried to make the getBufferSize()-calls-getBufferSizeFor() change that
you suggested, Jason, but it causes a lot of unit test failures, so there must
be something else
Github user cwestin commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-144898125
Parth:
Re ObjectVector: I don't know what that's for. I just followed the pattern:
getBufferSize() already throws that exception.
Re OUTPUT_MEMORY_LIMIT: what do y
Github user parthchandra commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-144897173
Looks good. +1.
Some minor comments in line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If yo
Github user cwestin commented on a diff in the pull request:
https://github.com/apache/drill/pull/181#discussion_r40985372
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java
---
@@ -123,6 +123,15 @@ public int getBufferSize() {
Github user jaltekruse commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-144846430
Overall, the changes look solid.
I am thinking it may be worth trying to remove the bodies of the existing
getBufferSize() methods and replace them with calls
Github user jaltekruse commented on a diff in the pull request:
https://github.com/apache/drill/pull/181#discussion_r40963410
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java
---
@@ -123,6 +123,15 @@ public int getBufferSize(
Github user cwestin commented on the pull request:
https://github.com/apache/drill/pull/181#issuecomment-144729913
Passes the unit tests, and the regression suite, except for one query in
the functional suite that Rahul claimed (in email) is a known problem and that
he has a fix for.
GitHub user cwestin opened a pull request:
https://github.com/apache/drill/pull/181
DRILL-3874: flattening large JSON objects uses too much direct memory
- add getBufferSizeFor() to ValueVector interface
- add implememtations of getBufferSizeFor() for all ValueVector derivatives
16 matches
Mail list logo
