Uploaded image for project: 'Tajo'
  1. Tajo
  2. TAJO-1782

Check ON_ERROR_STOP flag in TSQL when error is occured

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 0.11.0
    • Fix Version/s: 0.11.0
    • Component/s: SQL Shell
    • Labels:
      None

      Description

      When I enter query with abnormal part, TSQL print out error type and error message. and tsql is killed itself.

        Activity

        Hide
        dkhwangbo Dongkyu Hwangbo added a comment -

        Change the title in better.

        Show
        dkhwangbo Dongkyu Hwangbo added a comment - Change the title in better.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/708#issuecomment-134224201

        Looks good to me.
        @hyunsik, would you mind reviewing this patch? It looks to be related to your recent patches.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/708#issuecomment-134224201 Looks good to me. @hyunsik, would you mind reviewing this patch? It looks to be related to your recent patches.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/708#issuecomment-134870293

        The patch looks good to me. If possible, could you add some unit tests to guarantee the bug fix?

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/708#issuecomment-134870293 The patch looks good to me. If possible, could you add some unit tests to guarantee the bug fix?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkhwangbo closed the pull request at:

        https://github.com/apache/tajo/pull/708

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo closed the pull request at: https://github.com/apache/tajo/pull/708
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dkhwangbo opened a pull request:

        https://github.com/apache/tajo/pull/711

        TAJO-1782: Check ON_ERROR_STOP flag in TSQL when error is occured

        TSQL must check ON_ERROR_STOP flag when error is occured.
        When error is occured and ON_ERROR_STOP == true, TajoCli must be closed.
        but runshell() had no code about it, so add this operation.
        Unit test operation is added.

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

        $ git pull https://github.com/dkhwangbo/tajo TAJO-1782

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

        https://github.com/apache/tajo/pull/711.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 #711


        commit 76f92caabc7df786d0ec146dcc1a390207aa2c7d
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T05:38:16Z

        if at least one failed, check ON_ERROR_STOP flag

        commit b60899b60619c350eb592cf5fc9bc461543d6c99
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T09:36:41Z

        Modified. I misunderstood about it.

        commit 2308771061d179ed47546a58ac47df972c51873f
        Author: dkhwangbo <dk@dkhwangbo.(none)>
        Date: 2015-08-24T13:20:41Z

        Merge remote-tracking branch 'upstream/master'

        commit 2fbac04b336a43554d2c642813738175a1a25ffc
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T13:27:17Z

        Modify comments

        commit cf05863960b0b59c41c432c36c5e75459f10d411
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-26T07:28:07Z

        Merge remote-tracking branch 'upstream/master'

        commit 793ba87c42c8938a22a6b1da4745db5fc13a0002
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T02:03:03Z

        Merge remote-tracking branch 'upstream/master'

        commit c8062df760b950b40c35f9894df804839176c8cd
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T03:28:16Z

        add tajocli closing operation and Unit test

        commit 9225aac904fd3065eb9027dc4fa92032292413c1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T03:31:05Z

        remove unused class


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dkhwangbo opened a pull request: https://github.com/apache/tajo/pull/711 TAJO-1782 : Check ON_ERROR_STOP flag in TSQL when error is occured TSQL must check ON_ERROR_STOP flag when error is occured. When error is occured and ON_ERROR_STOP == true, TajoCli must be closed. but runshell() had no code about it, so add this operation. Unit test operation is added. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dkhwangbo/tajo TAJO-1782 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/711.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 #711 commit 76f92caabc7df786d0ec146dcc1a390207aa2c7d Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T05:38:16Z if at least one failed, check ON_ERROR_STOP flag commit b60899b60619c350eb592cf5fc9bc461543d6c99 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T09:36:41Z Modified. I misunderstood about it. commit 2308771061d179ed47546a58ac47df972c51873f Author: dkhwangbo <dk@dkhwangbo.(none)> Date: 2015-08-24T13:20:41Z Merge remote-tracking branch 'upstream/master' commit 2fbac04b336a43554d2c642813738175a1a25ffc Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T13:27:17Z Modify comments commit cf05863960b0b59c41c432c36c5e75459f10d411 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-26T07:28:07Z Merge remote-tracking branch 'upstream/master' commit 793ba87c42c8938a22a6b1da4745db5fc13a0002 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T02:03:03Z Merge remote-tracking branch 'upstream/master' commit c8062df760b950b40c35f9894df804839176c8cd Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T03:28:16Z add tajocli closing operation and Unit test commit 9225aac904fd3065eb9027dc4fa92032292413c1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T03:31:05Z remove unused class
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dkhwangbo reopened a pull request:

        https://github.com/apache/tajo/pull/711

        TAJO-1782: Check ON_ERROR_STOP flag in TSQL when error is occured

        TSQL must check ON_ERROR_STOP flag when error is occured.
        When error is occured and ON_ERROR_STOP == true, TajoCli must be closed.
        but runshell() had no code about it, so add this operation.
        Unit test operation is added.

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

        $ git pull https://github.com/dkhwangbo/tajo TAJO-1782

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

        https://github.com/apache/tajo/pull/711.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 #711


        commit 76f92caabc7df786d0ec146dcc1a390207aa2c7d
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T05:38:16Z

        if at least one failed, check ON_ERROR_STOP flag

        commit b60899b60619c350eb592cf5fc9bc461543d6c99
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T09:36:41Z

        Modified. I misunderstood about it.

        commit 2308771061d179ed47546a58ac47df972c51873f
        Author: dkhwangbo <dk@dkhwangbo.(none)>
        Date: 2015-08-24T13:20:41Z

        Merge remote-tracking branch 'upstream/master'

        commit 2fbac04b336a43554d2c642813738175a1a25ffc
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T13:27:17Z

        Modify comments

        commit cf05863960b0b59c41c432c36c5e75459f10d411
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-26T07:28:07Z

        Merge remote-tracking branch 'upstream/master'

        commit 793ba87c42c8938a22a6b1da4745db5fc13a0002
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T02:03:03Z

        Merge remote-tracking branch 'upstream/master'

        commit c8062df760b950b40c35f9894df804839176c8cd
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T03:28:16Z

        add tajocli closing operation and Unit test

        commit 9225aac904fd3065eb9027dc4fa92032292413c1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T03:31:05Z

        remove unused class

        commit 0b6a06dcf15674f57fe0d2cc63b9bdf3f16a1d08
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T02:15:25Z

        Correct misprints. unit test renewal.

        commit 11402e6efe4ef6d1cb59d767f2de6bd320d28da1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T02:18:13Z

        remove unused class

        commit eddfb01b0403d62c22113e572a6dce895d08c9d1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T02:22:40Z

        remove TIMEOUT


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dkhwangbo reopened a pull request: https://github.com/apache/tajo/pull/711 TAJO-1782 : Check ON_ERROR_STOP flag in TSQL when error is occured TSQL must check ON_ERROR_STOP flag when error is occured. When error is occured and ON_ERROR_STOP == true, TajoCli must be closed. but runshell() had no code about it, so add this operation. Unit test operation is added. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dkhwangbo/tajo TAJO-1782 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/711.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 #711 commit 76f92caabc7df786d0ec146dcc1a390207aa2c7d Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T05:38:16Z if at least one failed, check ON_ERROR_STOP flag commit b60899b60619c350eb592cf5fc9bc461543d6c99 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T09:36:41Z Modified. I misunderstood about it. commit 2308771061d179ed47546a58ac47df972c51873f Author: dkhwangbo <dk@dkhwangbo.(none)> Date: 2015-08-24T13:20:41Z Merge remote-tracking branch 'upstream/master' commit 2fbac04b336a43554d2c642813738175a1a25ffc Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T13:27:17Z Modify comments commit cf05863960b0b59c41c432c36c5e75459f10d411 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-26T07:28:07Z Merge remote-tracking branch 'upstream/master' commit 793ba87c42c8938a22a6b1da4745db5fc13a0002 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T02:03:03Z Merge remote-tracking branch 'upstream/master' commit c8062df760b950b40c35f9894df804839176c8cd Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T03:28:16Z add tajocli closing operation and Unit test commit 9225aac904fd3065eb9027dc4fa92032292413c1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T03:31:05Z remove unused class commit 0b6a06dcf15674f57fe0d2cc63b9bdf3f16a1d08 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T02:15:25Z Correct misprints. unit test renewal. commit 11402e6efe4ef6d1cb59d767f2de6bd320d28da1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T02:18:13Z remove unused class commit eddfb01b0403d62c22113e572a6dce895d08c9d1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T02:22:40Z remove TIMEOUT
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkhwangbo closed the pull request at:

        https://github.com/apache/tajo/pull/711

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo closed the pull request at: https://github.com/apache/tajo/pull/711
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkhwangbo closed the pull request at:

        https://github.com/apache/tajo/pull/711

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo closed the pull request at: https://github.com/apache/tajo/pull/711
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user dkhwangbo reopened a pull request:

        https://github.com/apache/tajo/pull/711

        TAJO-1782: Check ON_ERROR_STOP flag in TSQL when error is occured

        TSQL must check ON_ERROR_STOP flag when error is occured.
        When error is occured and ON_ERROR_STOP == true, TajoCli must be closed.
        but runshell() had no code about it, so add this operation.
        Unit test operation is added.

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

        $ git pull https://github.com/dkhwangbo/tajo TAJO-1782

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

        https://github.com/apache/tajo/pull/711.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 #711


        commit 76f92caabc7df786d0ec146dcc1a390207aa2c7d
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T05:38:16Z

        if at least one failed, check ON_ERROR_STOP flag

        commit b60899b60619c350eb592cf5fc9bc461543d6c99
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T09:36:41Z

        Modified. I misunderstood about it.

        commit 2308771061d179ed47546a58ac47df972c51873f
        Author: dkhwangbo <dk@dkhwangbo.(none)>
        Date: 2015-08-24T13:20:41Z

        Merge remote-tracking branch 'upstream/master'

        commit 2fbac04b336a43554d2c642813738175a1a25ffc
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-24T13:27:17Z

        Modify comments

        commit cf05863960b0b59c41c432c36c5e75459f10d411
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-26T07:28:07Z

        Merge remote-tracking branch 'upstream/master'

        commit 793ba87c42c8938a22a6b1da4745db5fc13a0002
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T02:03:03Z

        Merge remote-tracking branch 'upstream/master'

        commit c8062df760b950b40c35f9894df804839176c8cd
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T03:28:16Z

        add tajocli closing operation and Unit test

        commit 9225aac904fd3065eb9027dc4fa92032292413c1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-27T03:31:05Z

        remove unused class

        commit 0b6a06dcf15674f57fe0d2cc63b9bdf3f16a1d08
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T02:15:25Z

        Correct misprints. unit test renewal.

        commit 11402e6efe4ef6d1cb59d767f2de6bd320d28da1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T02:18:13Z

        remove unused class

        commit eddfb01b0403d62c22113e572a6dce895d08c9d1
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T02:22:40Z

        remove TIMEOUT

        commit cc768c3e680e22f3e02352410b64aac04d8516cc
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T03:58:37Z

        initialize exitCode.

        commit 29d108a607d81270c7e521675525d9376ee1208e
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-08-28T05:03:49Z

        fix internal error

        commit 26c7fe76c5161a101117b8dc165f2e8475d07f46
        Author: Dongkyu Hwangbo <dkhwangbo@gruter.com>
        Date: 2015-09-03T05:39:17Z

        Modified code. Test with thread.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user dkhwangbo reopened a pull request: https://github.com/apache/tajo/pull/711 TAJO-1782 : Check ON_ERROR_STOP flag in TSQL when error is occured TSQL must check ON_ERROR_STOP flag when error is occured. When error is occured and ON_ERROR_STOP == true, TajoCli must be closed. but runshell() had no code about it, so add this operation. Unit test operation is added. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dkhwangbo/tajo TAJO-1782 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/711.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 #711 commit 76f92caabc7df786d0ec146dcc1a390207aa2c7d Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T05:38:16Z if at least one failed, check ON_ERROR_STOP flag commit b60899b60619c350eb592cf5fc9bc461543d6c99 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T09:36:41Z Modified. I misunderstood about it. commit 2308771061d179ed47546a58ac47df972c51873f Author: dkhwangbo <dk@dkhwangbo.(none)> Date: 2015-08-24T13:20:41Z Merge remote-tracking branch 'upstream/master' commit 2fbac04b336a43554d2c642813738175a1a25ffc Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-24T13:27:17Z Modify comments commit cf05863960b0b59c41c432c36c5e75459f10d411 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-26T07:28:07Z Merge remote-tracking branch 'upstream/master' commit 793ba87c42c8938a22a6b1da4745db5fc13a0002 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T02:03:03Z Merge remote-tracking branch 'upstream/master' commit c8062df760b950b40c35f9894df804839176c8cd Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T03:28:16Z add tajocli closing operation and Unit test commit 9225aac904fd3065eb9027dc4fa92032292413c1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-27T03:31:05Z remove unused class commit 0b6a06dcf15674f57fe0d2cc63b9bdf3f16a1d08 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T02:15:25Z Correct misprints. unit test renewal. commit 11402e6efe4ef6d1cb59d767f2de6bd320d28da1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T02:18:13Z remove unused class commit eddfb01b0403d62c22113e572a6dce895d08c9d1 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T02:22:40Z remove TIMEOUT commit cc768c3e680e22f3e02352410b64aac04d8516cc Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T03:58:37Z initialize exitCode. commit 29d108a607d81270c7e521675525d9376ee1208e Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-08-28T05:03:49Z fix internal error commit 26c7fe76c5161a101117b8dc165f2e8475d07f46 Author: Dongkyu Hwangbo <dkhwangbo@gruter.com> Date: 2015-09-03T05:39:17Z Modified code. Test with thread.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/711#discussion_r38885046

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java —
        @@ -315,6 +315,38 @@ private void verifyStopWhenError() throws Exception

        { assertOutputResult(consoleResult); }

        + private void verifyRunWhenError() throws Exception {
        + Thread t = new Thread() {
        + public void run() {
        + try {
        + PipedOutputStream po = new PipedOutputStream();
        + InputStream is = new PipedInputStream(po);
        + ByteArrayOutputStream out = new ByteArrayOutputStream();
        +
        + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration();
        — End diff –

        It is better to call ```new TajoConf()``` because TpchTestBase.getInstance() starts a tajo cluster.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/711#discussion_r38885046 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java — @@ -315,6 +315,38 @@ private void verifyStopWhenError() throws Exception { assertOutputResult(consoleResult); } + private void verifyRunWhenError() throws Exception { + Thread t = new Thread() { + public void run() { + try { + PipedOutputStream po = new PipedOutputStream(); + InputStream is = new PipedInputStream(po); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration(); — End diff – It is better to call ```new TajoConf()``` because TpchTestBase.getInstance() starts a tajo cluster.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/711#discussion_r38885148

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java —
        @@ -315,6 +315,38 @@ private void verifyStopWhenError() throws Exception

        { assertOutputResult(consoleResult); }

        + private void verifyRunWhenError() throws Exception {
        + Thread t = new Thread() {
        + public void run() {
        + try {
        + PipedOutputStream po = new PipedOutputStream();
        + InputStream is = new PipedInputStream(po);
        + ByteArrayOutputStream out = new ByteArrayOutputStream();
        +
        + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration();
        + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName());
        + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out);
        +
        + tc.executeMetaCommand("
        set ON_ERROR_STOP false");
        + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false");
        +
        + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes());
        +
        + if(tc.runShell() == -1)
        + Thread.currentThread().interrupt();
        +
        + } catch (Exception e)

        { + e.printStackTrace(); + }

        + }
        + };
        +
        + t.start();
        + Thread.sleep(5000);
        — End diff –

        5 seconds seem to be too long. I think 1 second is enough.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/711#discussion_r38885148 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java — @@ -315,6 +315,38 @@ private void verifyStopWhenError() throws Exception { assertOutputResult(consoleResult); } + private void verifyRunWhenError() throws Exception { + Thread t = new Thread() { + public void run() { + try { + PipedOutputStream po = new PipedOutputStream(); + InputStream is = new PipedInputStream(po); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration(); + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName()); + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out); + + tc.executeMetaCommand(" set ON_ERROR_STOP false"); + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false"); + + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes()); + + if(tc.runShell() == -1) + Thread.currentThread().interrupt(); + + } catch (Exception e) { + e.printStackTrace(); + } + } + }; + + t.start(); + Thread.sleep(5000); — End diff – 5 seconds seem to be too long. I think 1 second is enough.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/711#discussion_r38885171

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java —
        @@ -315,6 +315,38 @@ private void verifyStopWhenError() throws Exception

        { assertOutputResult(consoleResult); }

        + private void verifyRunWhenError() throws Exception {
        + Thread t = new Thread() {
        + public void run() {
        + try {
        + PipedOutputStream po = new PipedOutputStream();
        + InputStream is = new PipedInputStream(po);
        + ByteArrayOutputStream out = new ByteArrayOutputStream();
        +
        + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration();
        + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName());
        + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out);
        +
        + tc.executeMetaCommand("
        set ON_ERROR_STOP false");
        + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false");
        +
        + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes());
        +
        + if(tc.runShell() == -1)
        + Thread.currentThread().interrupt();
        +
        + } catch (Exception e)

        { + e.printStackTrace(); + }

        + }
        + };
        +
        + t.start();
        + Thread.sleep(5000);
        + if(!t.isAlive())
        + assertTrue(false);
        — End diff –

        It would be better if some error message is also printed.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/711#discussion_r38885171 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java — @@ -315,6 +315,38 @@ private void verifyStopWhenError() throws Exception { assertOutputResult(consoleResult); } + private void verifyRunWhenError() throws Exception { + Thread t = new Thread() { + public void run() { + try { + PipedOutputStream po = new PipedOutputStream(); + InputStream is = new PipedInputStream(po); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration(); + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName()); + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out); + + tc.executeMetaCommand(" set ON_ERROR_STOP false"); + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false"); + + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes()); + + if(tc.runShell() == -1) + Thread.currentThread().interrupt(); + + } catch (Exception e) { + e.printStackTrace(); + } + } + }; + + t.start(); + Thread.sleep(5000); + if(!t.isAlive()) + assertTrue(false); — End diff – It would be better if some error message is also printed.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/711#discussion_r38885193

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java —
        @@ -362,6 +394,11 @@ public void testStopWhenError() throws Exception {
        }

        @Test
        + public void testRunWhenError() throws Exception {
        + verifyRunWhenError();
        — End diff –

        It seems that you don't have to write a new separate method and call it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/711#discussion_r38885193 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java — @@ -362,6 +394,11 @@ public void testStopWhenError() throws Exception { } @Test + public void testRunWhenError() throws Exception { + verifyRunWhenError(); — End diff – It seems that you don't have to write a new separate method and call it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user dkhwangbo commented on the pull request:

        https://github.com/apache/tajo/pull/711#issuecomment-138422239

        @jihoonson
        Thanks of your review. I reflect your comment.
        Please review again.

        Show
        githubbot ASF GitHub Bot added a comment - Github user dkhwangbo commented on the pull request: https://github.com/apache/tajo/pull/711#issuecomment-138422239 @jihoonson Thanks of your review. I reflect your comment. Please review again.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/711#discussion_r38904214

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java —
        @@ -362,6 +362,39 @@ public void testStopWhenError() throws Exception {
        }

        @Test
        + public void testRunWhenError() throws Exception {
        + Thread t = new Thread() {
        + public void run() {
        + try {
        + PipedOutputStream po = new PipedOutputStream();
        + InputStream is = new PipedInputStream(po);
        + ByteArrayOutputStream out = new ByteArrayOutputStream();
        +
        + TajoConf tajoConf = new TajoConf();
        + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName());
        + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out);
        +
        + tc.executeMetaCommand("
        set ON_ERROR_STOP false");
        + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false");
        +
        + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes());
        +
        + tc.runShell();
        + } catch (Exception e) {
        + e.printStackTrace();
        — End diff –

        It would be good to throw Exception properly than just printing it.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/711#discussion_r38904214 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java — @@ -362,6 +362,39 @@ public void testStopWhenError() throws Exception { } @Test + public void testRunWhenError() throws Exception { + Thread t = new Thread() { + public void run() { + try { + PipedOutputStream po = new PipedOutputStream(); + InputStream is = new PipedInputStream(po); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + TajoConf tajoConf = new TajoConf(); + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName()); + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out); + + tc.executeMetaCommand(" set ON_ERROR_STOP false"); + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false"); + + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes()); + + tc.runShell(); + } catch (Exception e) { + e.printStackTrace(); — End diff – It would be good to throw Exception properly than just printing it.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on a diff in the pull request:

        https://github.com/apache/tajo/pull/711#discussion_r38904299

        — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java —
        @@ -362,6 +362,39 @@ public void testStopWhenError() throws Exception {
        }

        @Test
        + public void testRunWhenError() throws Exception {
        + Thread t = new Thread() {
        + public void run() {
        + try {
        + PipedOutputStream po = new PipedOutputStream();
        + InputStream is = new PipedInputStream(po);
        + ByteArrayOutputStream out = new ByteArrayOutputStream();
        +
        + TajoConf tajoConf = new TajoConf();
        + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName());
        + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out);
        +
        + tc.executeMetaCommand("
        set ON_ERROR_STOP false");
        + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false");
        +
        + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes());
        +
        + tc.runShell();
        + } catch (Exception e)

        { + e.printStackTrace(); + }

        + }
        + };
        +
        + t.start();
        + Thread.sleep(1000);
        + if(!t.isAlive())
        — End diff –

        Our coding convention needs curly braces for ```if``` clause.

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/711#discussion_r38904299 — Diff: tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java — @@ -362,6 +362,39 @@ public void testStopWhenError() throws Exception { } @Test + public void testRunWhenError() throws Exception { + Thread t = new Thread() { + public void run() { + try { + PipedOutputStream po = new PipedOutputStream(); + InputStream is = new PipedInputStream(po); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + TajoConf tajoConf = new TajoConf(); + setVar(tajoCli, SessionVars.CLI_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName()); + TajoCli tc = new TajoCli(tajoConf, new String[]{}, is, out); + + tc.executeMetaCommand(" set ON_ERROR_STOP false"); + assertSessionVar(tc, SessionVars.ON_ERROR_STOP.keyname(), "false"); + + po.write(new String("asdf;\nqwe;\nzxcv;\n").getBytes()); + + tc.runShell(); + } catch (Exception e) { + e.printStackTrace(); + } + } + }; + + t.start(); + Thread.sleep(1000); + if(!t.isAlive()) — End diff – Our coding convention needs curly braces for ```if``` clause.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user jihoonson commented on the pull request:

        https://github.com/apache/tajo/pull/711#issuecomment-138816153

        +1 LGTM!

        Show
        githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/711#issuecomment-138816153 +1 LGTM!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/tajo/pull/711

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/711
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Tajo-master-CODEGEN-build #494 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/494/)
        TAJO-1782: Check ON_ERROR_STOP flag in TSQL when error is occured. (jihoonson: rev 745818b7f921b2b3bc400cdc272526e132ec4ac9)

        • tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java
        • CHANGES
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #494 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/494/ ) TAJO-1782 : Check ON_ERROR_STOP flag in TSQL when error is occured. (jihoonson: rev 745818b7f921b2b3bc400cdc272526e132ec4ac9) tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java CHANGES
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #851 (See https://builds.apache.org/job/Tajo-master-build/851/)
        TAJO-1782: Check ON_ERROR_STOP flag in TSQL when error is occured. (jihoonson: rev 745818b7f921b2b3bc400cdc272526e132ec4ac9)

        • CHANGES
        • tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #851 (See https://builds.apache.org/job/Tajo-master-build/851/ ) TAJO-1782 : Check ON_ERROR_STOP flag in TSQL when error is occured. (jihoonson: rev 745818b7f921b2b3bc400cdc272526e132ec4ac9) CHANGES tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java
        Hide
        jihoonson Jihoon Son added a comment -

        Dongkyu Hwangbo, thanks for your work. Your patch is committed to master and branch-0.11.
        Maybe this is your first commit. Congratulation!

        Show
        jihoonson Jihoon Son added a comment - Dongkyu Hwangbo , thanks for your work. Your patch is committed to master and branch-0.11. Maybe this is your first commit. Congratulation!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-0.11.0-build #28 (See https://builds.apache.org/job/Tajo-0.11.0-build/28/)
        TAJO-1782: Check ON_ERROR_STOP flag in TSQL when error is occured. (jihoonson: rev c13b0562f39f7bbbcf4ffd073f469c32962e1190)

        • tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java
        • CHANGES
        • tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-0.11.0-build #28 (See https://builds.apache.org/job/Tajo-0.11.0-build/28/ ) TAJO-1782 : Check ON_ERROR_STOP flag in TSQL when error is occured. (jihoonson: rev c13b0562f39f7bbbcf4ffd073f469c32962e1190) tajo-core-tests/src/test/java/org/apache/tajo/cli/tsql/TestTajoCli.java CHANGES tajo-cli/src/main/java/org/apache/tajo/cli/tsql/TajoCli.java

          People

          • Assignee:
            dkhwangbo Dongkyu Hwangbo
            Reporter:
            dkhwangbo Dongkyu Hwangbo
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development