asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in hyracks[master]: Add flush() to IFrameWriter
Date Mon, 25 Jan 2016 11:15:13 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Add flush() to IFrameWriter
......................................................................


Patch Set 5:

(20 comments)

https://asterix-gerrit.ics.uci.edu/#/c/584/5/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamProjectRuntimeFactory.java
File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamProjectRuntimeFactory.java:

Line 90:                 writer.flush();
> appender.flush()?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamSelectRuntimeFactory.java
File algebricks/algebricks-runtime/src/main/java/org/apache/hyracks/algebricks/runtime/operators/std/StreamSelectRuntimeFactory.java:

Line 164:                 writer.flush();
> appender.flush()?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameAppender.java
File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IFrameAppender.java:

Line 51:      * Flush the frame content to the given writer.
> Flush-->Write
Done


Line 52:      * Clear the inner buffer after flush if {@code clear} is <code>true</code>.
> flush-->write
Done


Line 58:     void write(IFrameWriter outWriter, boolean clear) throws HyracksDataException;
> The javadoc comments for write and flush don't align very well with the met
Done


Line 65:     public default void flush(IFrameWriter writer) throws HyracksDataException {
> do you need the clear bit parameter here?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/FrameTupleAppenderWrapper.java
File hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/comm/io/FrameTupleAppenderWrapper.java:

Line 49:     public void flush() throws HyracksDataException {
> The meaning of this flush() call meant "write".
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/group/preclustered/PreclusteredGroupWriter.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/group/preclustered/PreclusteredGroupWriter.java:

Line 190:         // operator should only send its output once all of its input has been consumed.
hence, this is a no op
> throw an exception here if this method should never be called.
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/HybridHashJoinOperatorDescriptor.java:

Line 577:                     // flush() on any join operator is a no op
> throw an exception here
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/InMemoryHashJoinOperatorDescriptor.java:

Line 257:                     // flush() on any join operator is a no op
> throw an exception here.
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/NestedLoopJoinOperatorDescriptor.java:

Line 222: 
> throw an exception here.
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/join/OptimizedHybridHashJoinOperatorDescriptor.java:

Line 126:     // Flags added for test purpose
> It seems your format is different?  I tried to format this format in my Ecl
I tried customizing it to get rid of white spaces completely but it seemed to create too many
changes in comments. I have reverted it now.

Done.


Line 680:                     // flush() on any join operator is a no op
> throw an exception here.
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/MaterializingOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/MaterializingOperatorDescriptor.java:

Line 121:                 }
> throw an exception?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/PrinterOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/PrinterOperatorDescriptor.java:

Line 68:             // flush() on data writer operators is a no op
> why data writer operator is a no op?
done.


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/SplitVectorOperatorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/misc/SplitVectorOperatorDescriptor.java:

Line 113:                     // flush() on data writer operators is a no op
> why this is a no op?
Fixed the comment. this is the collect part which I think don't make sense to call flush()
on since its sole job is to deliver data to some buffer.


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/ExternalSortRunGenerator.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/ExternalSortRunGenerator.java:

Line 120:         // flush() on sort operators is a no op since the sort needs all of its
input before sending any output
> throw an exception?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/HeapSortRunGenerator.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/HeapSortRunGenerator.java:

Line 104:         // flush() on sort operators is a no op since the sort needs all of its
input before sending any output
> throw an exception?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/dataflow/IndexSearchOperatorNodePushable.java:

Line 193:         writer.flush();
> appender.flush()?
Done


https://asterix-gerrit.ics.uci.edu/#/c/584/5/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/BinaryTokenizerOperatorNodePushable.java
File hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/dataflow/BinaryTokenizerOperatorNodePushable.java:

Line 172:         }
> appender.flush()?
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/584
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I85424bab7965b71aac709280af066e1655457aa3
Gerrit-PatchSet: 5
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hubailmor@gmail.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message