Conversation
|
@clintropolis @vogievetsky Since the |
|
Would you please check the build failure? It looks legit. |
| throw new NoSuchElementException(); | ||
| } | ||
| if (!parsedInputRows.isEmpty()) { | ||
| return parsedInputRows.remove(0); |
There was a problem hiding this comment.
ArrayList.remove() internally moves the positions of remaining elements and this will happen for every row in this case which could be expensive. Since parsedInputRows won't be large in general, I suggest to use an iterator on the list instead of modifying the list.
public E remove(int index) {
rangeCheck(index);
modCount++;
E oldValue = elementData(index);
int numMoved = size - index - 1;
if (numMoved > 0)
System.arraycopy(elementData, index+1, elementData, index,
numMoved);
elementData[--size] = null; // clear to let GC do its work
return oldValue;
}There was a problem hiding this comment.
Cool, it's a good idea, I was worrying about this issue too.
Tried to use LinkedList so that's O(1) pop, but I wonder why it's a forbidden API?
There was a problem hiding this comment.
It's because usually LinkedList is not a good idea. See #6112.
| }; | ||
| } | ||
|
|
||
| public interface ExploderMaker<T> |
There was a problem hiding this comment.
Would you please add javadoc to this class and methods? For class, It should include what this is and where it is used. For methods, it should say what the method does, what inputs and outputs are.
| { | ||
| List<Object> getExplodeArray(T node, String expr); | ||
|
|
||
| T setObj(T node, Object value, String expr); |
There was a problem hiding this comment.
I.. don't know what the best name is for this method, but setObj() sounds not intuitive enough. Maybe createNewNodeWithValue()? Or you can split this into two methods of copy() and setValue().
There was a problem hiding this comment.
I will think of more intuitive names for those two methods.
| null | ||
| ), | ||
| new JSONPathSpec(true, ImmutableList.of()), | ||
| null, |
There was a problem hiding this comment.
Should there be a sampler end-to-end unit test with an explodeSpec?
|
@SEKIRO-J this has a bunch of conflicts |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions. |
|
This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
5 participants
Description
Add explodeSpec feature, to give user ability to explode array objects without writing their own pre-processing scripts.
trying to add a forbidden-apis
in
FileIteratingFirehose, the fieldprivate final ArrayList<InputRow> parsedInputRows;should be better declared asLinkedList, as consequential operation involves lots ofpushandpop, so better to use a queue.ArrayDequedoesn't work here because we need to insertnullThis PR has:
Key changed/added classes in this PR
JSONExplodeSpecJSONExploderMakerObjectExploderObjectExploders