You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Paul Rogers (Code Review)" <ge...@cloudera.org> on 2018/11/30 23:20:51 UTC

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12016


Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................

IMPALA-7915: Wrap SQL parser to avoid redundant code

The FE has several repeated blocks of code to set up the lexer and
parser, to parse, and to handle errors. This patch moves this code
into a static function that can be used in place of the copies.

At the same time, provide a specific ParseException to replace the
generic Exception thrown by the parser to allow easier error handling.

Some of the uses of the parser assume the return value is Object,
others that the value is ParseNode and still others that it is
StatementBase. Since the actual return is StatementBase, declares
that as the return value of the new static method to clearly state the
actual output.

Testing: This is just a refactoring. Reran all FE tests to ensure no
regressions.

Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
---
A fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
8 files changed, 91 insertions(+), 59 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/12016/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12016/1/fe/src/main/java/org/apache/impala/analysis/Parser.java
File fe/src/main/java/org/apache/impala/analysis/Parser.java:

http://gerrit.cloudera.org:8080/#/c/12016/1/fe/src/main/java/org/apache/impala/analysis/Parser.java@41
PS1, Line 41:   public static StatementBase parse(String query, TQueryOptions options) throws ParseException {
line too long (96 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 23:21:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 3:

Failure seems to be in one of the Python tests:

=================================== FAILURES ===================================
_______________ TestImpalaShellInteractive.test_malformed_query ________________
[gw14] linux2 -- Python 2.7.12 /home/ubuntu/Impala/bin/../infra/python/env/bin/python
shell/test_shell_interactive.py:725: in test_malformed_query
    assert "ERROR: AnalysisException: Unmatched string literal" in result.stderr
E   assert 'ERROR: AnalysisException: Unmatched string literal' in "Starting Impala Shell without Kerberos authentication\nOpened TCP connection to localhost:21000\nConnected to localho...select foo('\\\\'), ('bar\n                    ^\n\nCAUSED BY: Exception: Unmatched string literal\n\nGoodbye ubuntu\n"
E    +  where "Starting Impala Shell without Kerberos authentication\nOpened TCP connection to localhost:21000\nConnected to localho...select foo('\\\\'), ('bar\n                    ^\n\nCAUSED BY: Exception: Unmatched string literal\n\nGoodbye ubuntu\n" = <tests.shell.util.ImpalaShellResult object at 0x7f17bd64cc10>.stderr

Anybody know about this one?


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 04:00:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 21:49:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 21:48:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1488/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 30 Nov 2018 23:54:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Dec 2018 21:30:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3560/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Dec 2018 21:30:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 2: Code-Review+2

Thanks for the clean up.


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Dec 2018 21:30:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 3:

That looks related to your change. AnalysException -> ParseException stuff.


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 06:32:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 3: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3560/


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Dec 2018 01:29:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1493/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 01 Dec 2018 00:28:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................

IMPALA-7915: Wrap SQL parser to avoid redundant code

The FE has several repeated blocks of code to set up the lexer and
parser, to parse, and to handle errors. This patch moves this code
into a static function that can be used in place of the copies.

At the same time, provide a specific ParseException to replace the
generic Exception thrown by the parser to allow easier error handling.

Some of the uses of the parser assume the return value is Object,
others that the value is ParseNode and still others that it is
StatementBase. Since the actual return is StatementBase, declares
that as the return value of the new static method to clearly state the
actual output.

Testing: This is just a refactoring. Reran all FE tests to ensure no
regressions.

Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Reviewed-on: http://gerrit.cloudera.org:8080/12016
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
A fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M tests/shell/test_shell_interactive.py
9 files changed, 95 insertions(+), 62 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 6
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Sat, 15 Dec 2018 01:38:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12016

to look at the new patch set (#2).

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................

IMPALA-7915: Wrap SQL parser to avoid redundant code

The FE has several repeated blocks of code to set up the lexer and
parser, to parse, and to handle errors. This patch moves this code
into a static function that can be used in place of the copies.

At the same time, provide a specific ParseException to replace the
generic Exception thrown by the parser to allow easier error handling.

Some of the uses of the parser assume the return value is Object,
others that the value is ParseNode and still others that it is
StatementBase. Since the actual return is StatementBase, declares
that as the return value of the new static method to clearly state the
actual output.

Testing: This is just a refactoring. Reran all FE tests to ensure no
regressions.

Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
---
A fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
8 files changed, 92 insertions(+), 59 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/12016/2
-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1617/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 21:47:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12016 )

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3568/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Comment-Date: Fri, 14 Dec 2018 21:49:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7915: Wrap SQL parser to avoid redundant code

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12016

to look at the new patch set (#4).

Change subject: IMPALA-7915: Wrap SQL parser to avoid redundant code
......................................................................

IMPALA-7915: Wrap SQL parser to avoid redundant code

The FE has several repeated blocks of code to set up the lexer and
parser, to parse, and to handle errors. This patch moves this code
into a static function that can be used in place of the copies.

At the same time, provide a specific ParseException to replace the
generic Exception thrown by the parser to allow easier error handling.

Some of the uses of the parser assume the return value is Object,
others that the value is ParseNode and still others that it is
StatementBase. Since the actual return is StatementBase, declares
that as the return value of the new static method to clearly state the
actual output.

Testing: This is just a refactoring. Reran all FE tests to ensure no
regressions.

Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
---
A fe/src/main/java/org/apache/impala/analysis/Parser.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/catalog/View.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/service/JdbcTest.java
M tests/shell/test_shell_interactive.py
9 files changed, 95 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/16/12016/4
-- 
To view, visit http://gerrit.cloudera.org:8080/12016
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I174c59d38542ff311c6c3dc10cf3ad4e40f8b30e
Gerrit-Change-Number: 12016
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>