----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13781/#review25964 -----------------------------------------------------------
src/org/apache/pig/PigToStream.java <https://reviews.apache.org/r/13781/#comment50670> No code change, remove this section from patch src/org/apache/pig/StreamToPig.java <https://reviews.apache.org/r/13781/#comment50671> No code change, remove this section from patch src/org/apache/pig/builtin/PigStreaming.java <https://reviews.apache.org/r/13781/#comment50672> No code change, remove this section from patch src/org/apache/pig/builtin/PigToStreamUDF.java <https://reviews.apache.org/r/13781/#comment50675> I feel those classes StreamingUDF should go to package org.apache.pig.impl.builtin, StreamingDelimiters, StreamUDFToPig, PigToStreamUDF, etc should go to org.apache.pig.impl.streaming src/org/apache/pig/builtin/PigToStreamUDF.java <https://reviews.apache.org/r/13781/#comment50673> I personally like to move the last element processing to the for loop: for (int i=0; i < sz; i++) { field = t.get(i); StorageUtil.putField(out, field, DELIMS, true); if (i!=sz-1) { out.write(DELIMS.getParamDelim()); } } src/org/apache/pig/builtin/StreamUDFToPig.java <https://reviews.apache.org/r/13781/#comment50684> This might be affected by PIG-3255 src/org/apache/pig/builtin/StreamUDFToPig.java <https://reviews.apache.org/r/13781/#comment50686> How do we handle delimit escaping? Seems we don't handle it right now? src/org/apache/pig/builtin/StreamingUDF.java <https://reviews.apache.org/r/13781/#comment50685> Are those files only for illustrate? Why not just use a random tmp file? Why do we distinguish exectype in illustrate? It only run locally. src/org/apache/pig/builtin/StreamingUDF.java <https://reviews.apache.org/r/13781/#comment50680> Any reason not to create a StreamingUDFInputHandler? src/org/apache/pig/builtin/StreamingUDF.java <https://reviews.apache.org/r/13781/#comment50679> Can we use FileUtils.copyFile? src/org/apache/pig/builtin/StreamingUDF.java <https://reviews.apache.org/r/13781/#comment50683> Seems flush() is more proper to put inside inputHandler src/org/apache/pig/impl/streaming/InputHandler.java <https://reviews.apache.org/r/13781/#comment50682> Why we need this change? test/unit-tests <https://reviews.apache.org/r/13781/#comment50681> This is not true unit test, should remove from the list Overall, the patch is non-intrusive. Except for minor refactory in ExecutableManager, existing features should not be affected. I am fine to check in code quickly, then improve the code over time. Before that, we need: * address existing comments, especially conflict with PIG-3255 * add some e2e tests * run regression tests - Daniel Dai On Sept. 5, 2013, 8:23 p.m., Jeremy Karn wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13781/ > ----------------------------------------------------------- > > (Updated Sept. 5, 2013, 8:23 p.m.) > > > Review request for pig. > > > Repository: pig-git > > > Description > ------- > > Changes for PIG-2417 (https://issues.apache.org/jira/browse/PIG-2417) > > > Diffs > ----- > > build.xml 7e22192 > src/org/apache/pig/PigToStream.java 7cc2950 > src/org/apache/pig/StreamToPig.java ff24b27 > src/org/apache/pig/builtin/PigStreaming.java 5467693 > src/org/apache/pig/builtin/PigToStreamUDF.java PRE-CREATION > src/org/apache/pig/builtin/StreamUDFToPig.java PRE-CREATION > src/org/apache/pig/builtin/StreamingDelimiters.java PRE-CREATION > src/org/apache/pig/builtin/StreamingUDF.java PRE-CREATION > src/org/apache/pig/builtin/StreamingUDFException.java PRE-CREATION > src/org/apache/pig/builtin/StreamingUDFOutputSchemaException.java > PRE-CREATION > src/org/apache/pig/impl/streaming/DefaultInputHandler.java 301bea3 > src/org/apache/pig/impl/streaming/DefaultOutputHandler.java 1b46e7d > src/org/apache/pig/impl/streaming/ExecutableManager.java cf79c83 > src/org/apache/pig/impl/streaming/InputHandler.java 690d94e > src/org/apache/pig/impl/streaming/OutputHandler.java 6e9262a > src/org/apache/pig/impl/streaming/StreamingUDFOutputHandler.java > PRE-CREATION > src/org/apache/pig/impl/streaming/StreamingUtil.java PRE-CREATION > src/org/apache/pig/impl/util/JarManager.java 5c4acb0 > src/org/apache/pig/impl/util/StorageUtil.java dcb62ec > src/org/apache/pig/scripting/ScriptEngine.java 29a9e1f > src/org/apache/pig/scripting/ScriptingIllustrateOutputCapturer.java > PRE-CREATION > src/org/apache/pig/scripting/streaming/python/PythonScriptEngine.java > PRE-CREATION > src/python/streaming/controller.py PRE-CREATION > src/python/streaming/pig_util.py PRE-CREATION > test/org/apache/pig/builtin/TestPigToStreamUDF.java PRE-CREATION > test/org/apache/pig/builtin/TestStreamUDFToPig.java PRE-CREATION > test/org/apache/pig/builtin/TestStreamingUDF.java PRE-CREATION > test/org/apache/pig/impl/streaming/TestExecutableManager.java 6246019 > test/org/apache/pig/impl/streaming/TestStreamingUDFOutputHandler.java > PRE-CREATION > test/org/apache/pig/impl/streaming/TestStreamingUtil.java PRE-CREATION > test/org/apache/pig/test/TestPigStreaming.java PRE-CREATION > test/org/apache/pig/test/TestStreaming.java 1eac5d2 > test/python/streaming/test_controller.py PRE-CREATION > test/unit-tests d52ad9d > > Diff: https://reviews.apache.org/r/13781/diff/ > > > Testing > ------- > > > Thanks, > > Jeremy Karn > >
