You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Minghui Zhu (Code Review)" <ge...@cloudera.org> on 2022/09/21 03:43:01 UTC
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Minghui Zhu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19023
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL, and there will be more requirements related to SQL parsing in the future. Before, we used jsqlParser for parsing. In order to be more compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide complete parsing capabilities, it is also necessary to support the toSql method of other statements, such as AlterTableStmt. But in our scenario, there are only select and use statements, which are already supported. So all we need to do is to analyze all the functions in sql and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase node = Parser.parse(sql);
FunctionCallExprVisitor functionCallExprVisitor = new FunctionCallExprVisitor();
node.accept(functionCallExprVisitor);
List<FunctionCallExpr> funcCalls = functionCallExprVisitor.getFuncCalls();
// do something with funcCalla
return node.toSql();
}
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
A fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java
A fe/src/main/java/org/apache/impala/util/StmtVisitor.java
9 files changed, 162 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/1
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 1
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11395/ : 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/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 1
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Sep 2022 04:04:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11962/ : 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/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 4
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 05 Dec 2022 08:10:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL,
and there will be more requirements related to SQL parsing in the
future. Before, we used jsqlParser for parsing. In order to be more
compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide
complete parsing capabilities, it is also necessary to support the
toSql method of other statements, such as AlterTableStmt. But in our
scenario, there are only select and use statements, which are already
supported. So all we need to do is to analyze all the functions in sql
and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase stmt = Parser.parse(sql);
ExprRewriter exprRewriter = new MyExprRewritter();
stmt.rewriteExprs(exprRewriter);
return stmt.toSql();
}
The MyExprRewriter class is not a common requirement, so it was not
submitted. This submission is mainly to support the construction of
the parser package.
It should be noted that it must be set in advance:
TBackendGflags tBackendGflags = new TBackendGflags();
tBackendGflags.setSkip_lookup_symbol(true);
BackendConfig.create(tBackendGflags);
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
8 files changed, 73 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/8
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 8
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 2:
(5 comments)
Thanks for working on this! I think this will benefit a lot for the community.
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG@9
PS2, Line 9: We have a requirement to rewrite the parameters of a function in SQL, and there will be more requirements related to SQL parsing in the future. Before, we used jsqlParser for parsing. In order to be more compatible with impala sql, we want to use impala's parser.
:
: Not all statements currently support the toSql method, so to provide complete parsing capabilities, it is also necessary to support the toSql method of other statements, such as AlterTableStmt. But in our scenario, there are only select and use statements, which are already supported. So all we need to do is to analyze all the functions in sql and type out the parser package.
nit: please wrap each line to have 72 or fewer characters if possible.
https://cwiki.apache.org/confluence/display/IMPALA/Contributing+to+Impala
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG@25
PS2, Line 25: FunctionCallExprVisitor functionCallExprVisitor = new FunctionCallExprVisitor();
: node.accept(functionCallExprVisitor);
: List<FunctionCallExpr> funcCalls = functionCallExprVisitor.getFuncCalls();
: // do something with funcCalla
I prefer using the existing ExprRewriter for customized expr rewrites. Here is the entrance:
https://github.com/apache/impala/blob/d47d305bf42f963ba6fac18c014e96c734e9e9ff/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L561
The pros is each statement already implements the rewriteExprs() interface that gets a ExprRewriter:
https://github.com/apache/impala/blob/3554b0752d4be142dabb82b6b751323cde5323d8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java#L128
If so, the code looks like
public String parse(String sql) {
StatementBase stmt = Parser.parse(sql);
ExprRewriter exprRewriter = new MyExprRewritter();
stmt.rewriteExprs(exprRewriter);
return stmt.toSql();
}
We will need to remove some preconditions like "Preconditions.checkState(isAnalyzed())"
https://github.com/apache/impala/blob/b91aa065377a1d154cd9eb5f5cd9ffb6da919b65/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java#L1382
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml
File fe/pom.xml:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml@1130
PS2, Line 1130: <id>impala-parser</id>
Should we activate this by default?
<activation>
<activeByDefault>true</activeByDefault>
</activation>
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml@1149
PS2, Line 1149: impala-parser
Can we append the impala version at the end? Something like impala-parser-4.2.0-RELEASE.jar makes more sense to me.
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS2, Line 82: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipLookupSymbol()) {
> what is skipLookupSymbol() for?
Same question from me. If we do need such a feature, we can add new methods like createBuiltinWithoutResolvingSymbols individually. Or do we need this method being used without resolving the symbols?
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 2
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Tue, 25 Oct 2022 07:38:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL,
and there will be more requirements related to SQL parsing in the
future. Before, we used jsqlParser for parsing. In order to be more
compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide
complete parsing capabilities, it is also necessary to support the
toSql method of other statements, such as AlterTableStmt. But in our
scenario, there are only select and use statements, which are already
supported. So all we need to do is to analyze all the functions in sql
and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase stmt = Parser.parse(sql);
ExprRewriter exprRewriter = new MyExprRewritter();
stmt.rewriteExprs(exprRewriter);
return stmt.toSql();
}
The MyExprRewriter class is not a common requirement, so it was not
submitted. This submission is mainly to support the construction of
the parser package.
It should be noted that it must be set in advance:
TBackendGflags tBackendGflags = new TBackendGflags();
tBackendGflags.setSkip_lookup_symbol(true);
BackendConfig.create(tBackendGflags);
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
8 files changed, 71 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/6
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 6
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11396/ : 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/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 2
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Sep 2022 04:12:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java
File fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java@34
PS2, Line 34: public class FunctionCallExprVisitor implements StmtVisitor<StmtNode> {
> when will the FunctionCallExprVisitor be used?
in the other project outside impala where we use parser to find all functions in the sql, would write like this:
StatementBase node = Parser.parse(sql);
FunctionCallExprVisitor functionCallExprVisitor = new FunctionCallExprVisitor();
node.accept(functionCallExprVisitor);
List<FunctionCallExpr> funcCalls = functionCallExprVisitor.getFuncCalls();
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 2
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 28 Sep 2022 06:33:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL,
and there will be more requirements related to SQL parsing in the
future. Before, we used jsqlParser for parsing. In order to be more
compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide
complete parsing capabilities, it is also necessary to support the
toSql method of other statements, such as AlterTableStmt. But in our
scenario, there are only select and use statements, which are already
supported. So all we need to do is to analyze all the functions in sql
and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase stmt = Parser.parse(sql);
ExprRewriter exprRewriter = new MyExprRewritter();
stmt.rewriteExprs(exprRewriter);
return stmt.toSql();
}
The MyExprRewriter class is not a common requirement, so it was not
submitted. This submission is mainly to support the construction of
the parser package.
It should be noted that it must be set in advance:
TBackendGflags tBackendGflags = new TBackendGflags();
tBackendGflags.setSkip_lookup_symbol(true);
BackendConfig.create(tBackendGflags);
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
6 files changed, 57 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/4
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 4
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Anonymous Coward (770) has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 9:
Is it capable of parsing multiple SQL statements, such as SELECT 1; SELECT 2?
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 9
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward (770)
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Tue, 15 Aug 2023 13:04:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 4:
(6 comments)
Thanks for updating the patch! I just have concern on adding the new flag, since it exposes unrelated internal info to the impala-parser user. Adding more reviewers to get their ideas.
http://gerrit.cloudera.org:8080/#/c/19023/4/common/thrift/BackendGflags.thrift
File common/thrift/BackendGflags.thrift:
http://gerrit.cloudera.org:8080/#/c/19023/4/common/thrift/BackendGflags.thrift@233
PS4, Line 233: skip_lookup_symbol
nit: "skip_symbol_lookup_in_builtins" is more clear for me
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml
File fe/pom.xml:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml@1130
PS2, Line 1130: </goals>
> This profile is generally used by other projects. When building a package,
I'm thinking about adding an example/test like this https://github.com/stiga-huang/impala-parser-helloworld . So would need it be generated by default. The example/test can avoid future changes break the basic usage of this jar. What's your opinion?
http://gerrit.cloudera.org:8080/#/c/19023/4/fe/pom.xml
File fe/pom.xml:
http://gerrit.cloudera.org:8080/#/c/19023/4/fe/pom.xml@1134
PS4, Line 1134: impala-minimal-hive-exec
Could you explain why we need impala-minimal-hive-exec in parsing a query string? It'd be nice to add a comment for it.
http://gerrit.cloudera.org:8080/#/c/19023/4/fe/pom.xml@1136
PS4, Line 1136: </artifactSet>
Can we exclude some classes that are not used in query parsing? E.g. I tried adding these
<filters>
<filter>
<artifact>org.apache.impala</artifact>
<excludes>
<exclude>org/apache/impala/catalog/events/**</exclude>
<exclude>org/apache/impala/catalog/local/**</exclude>
<exclude>org/apache/impala/catalog/metastore/**</exclude>
<exclude>org/apache/impala/catalog/monitor/**</exclude>
<exclude>org/apache/impala/planner/**</exclude>
</excludes>
</filter>
</filters>
The parser jar is still usable in my hello-world project: https://github.com/stiga-huang/impala-parser-helloworld
http://gerrit.cloudera.org:8080/#/c/19023/4/fe/pom.xml@1137
PS4, Line 1137: 4.2.0-RELEASE
We should not hard-code the version string here. Let's replaced it with ${project.version}
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS2, Line 82: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipLookupSymbol()) {
> The createBuiltin method will call the be code through the local method to
I see. Share the stacktrace I got without this skip BTW:
at org.apache.impala.util.NativeLibUtil.loadLibrary (NativeLibUtil.java:66)
at org.apache.impala.service.FeSupport.loadLibrary (FeSupport.java:491)
at org.apache.impala.service.FeSupport.LookupSymbol (FeSupport.java:213)
at org.apache.impala.service.FeSupport.LookupSymbol (FeSupport.java:222)
at org.apache.impala.catalog.Function.lookupSymbol (Function.java:475)
at org.apache.impala.catalog.ScalarFunction.createBuiltin (ScalarFunction.java:198)
at org.apache.impala.catalog.ScalarFunction.createBuiltinOperator (ScalarFunction.java:180)
at org.apache.impala.catalog.ScalarFunction.createBuiltinOperator (ScalarFunction.java:175)
at org.apache.impala.analysis.ArithmeticExpr.initBuiltins (ArithmeticExpr.java:105)
at org.apache.impala.catalog.BuiltinsDb.initBuiltins (BuiltinsDb.java:105)
at org.apache.impala.catalog.BuiltinsDb.<init> (BuiltinsDb.java:94)
at org.apache.impala.catalog.BuiltinsDb.getInstance (BuiltinsDb.java:79)
at org.apache.impala.analysis.SqlScanner.init (SqlScanner.java:750)
at org.apache.impala.analysis.SqlScanner.<clinit> (SqlScanner.java:766)
at org.apache.impala.analysis.Parser.parse (Parser.java:59)
at org.apache.impala.analysis.Parser.parse (Parser.java:42)
However, this seems a hack for me. It's not that maintainable since we might need more flags like this if we found other logics should be skipped in the future. We do have some flags for external frontend. But it seems none of them can skip this.
Generally, resolving the function names is part of the analyze phase. We should not init the builtin symbols when initializing the parser. But I don't have a better solution. Let's see if other reviewers have more ideas.
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 4
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Tue, 06 Dec 2022 09:34:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12611/ : 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/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 6
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 12:06:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Anonymous Coward (770) has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 9:
> > Patch Set 9:
> >
> > Is it capable of parsing multiple SQL statements, such as SELECT
> 1; SELECT 2?
>
> Hi yi-you, maybe you can split SQL statements by ';', and then pass
> them to the Parser one by one.
> > Patch Set 9:
> >
> > Is it capable of parsing multiple SQL statements, such as SELECT
> 1; SELECT 2?
>
> Hi yi-you, maybe you can split SQL statements by ';', and then pass
> them to the Parser one by one.
How can we split SQL based on the ';'? Would using regular expressions be appropriate? Ideally, parsing the entire block of SQL and determining the number of individual statements would be more precise. Tools such as jsqlparser and the sqlparse adopted by impala-shell are designed to handle multi-statement SQL.
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 9
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward (770)
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Thu, 17 Aug 2023 08:55:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19023/1/fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java
File fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java:
http://gerrit.cloudera.org:8080/#/c/19023/1/fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java@35
PS1, Line 35:
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 1
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Sep 2022 03:43:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19023/7/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java:
http://gerrit.cloudera.org:8080/#/c/19023/7/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS7, Line 82: if (BackendConfig.INSTANCE != null &&
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/19023/7/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@194
PS7, Line 194: if (BackendConfig.INSTANCE != null &&
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 7
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 11:50:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12614/ : 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/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 8
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 12:14:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19023
to look at the new patch set (#2).
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL, and there will be more requirements related to SQL parsing in the future. Before, we used jsqlParser for parsing. In order to be more compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide complete parsing capabilities, it is also necessary to support the toSql method of other statements, such as AlterTableStmt. But in our scenario, there are only select and use statements, which are already supported. So all we need to do is to analyze all the functions in sql and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase node = Parser.parse(sql);
FunctionCallExprVisitor functionCallExprVisitor = new FunctionCallExprVisitor();
node.accept(functionCallExprVisitor);
List<FunctionCallExpr> funcCalls = functionCallExprVisitor.getFuncCalls();
// do something with funcCalla
return node.toSql();
}
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/StmtNode.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
A fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java
A fe/src/main/java/org/apache/impala/util/StmtVisitor.java
9 files changed, 162 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/2
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 2
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Jian Zhang (Code Review)" <ge...@cloudera.org>.
Jian Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java
File fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/FunctionCallExprVisitor.java@34
PS2, Line 34: public class FunctionCallExprVisitor implements StmtVisitor<StmtNode> {
when will the FunctionCallExprVisitor be used?
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 2
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Comment-Date: Thu, 22 Sep 2022 03:42:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19023/6/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java:
http://gerrit.cloudera.org:8080/#/c/19023/6/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS6, Line 82: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipSymbolLookupInBuiltins()) {
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/19023/6/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@193
PS6, Line 193: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipSymbolLookupInBuiltins()) {
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/19023/6/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:
http://gerrit.cloudera.org:8080/#/c/19023/6/fe/src/main/java/org/apache/impala/service/BackendConfig.java@393
PS6, Line 393:
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 6
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 11:46:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 7:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/12612/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 7
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 12:02:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 4:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG@1
PS2, Line 1: Parent: 2e6bbbba (IMPALA-11767: Ignore exceptions for invalid paths in Hudi search)
> can you add some UT?
The visitor code has been deleted, and the code logic has not been modified, but a parser package has been built, so there is no need to add a unit test, or do you have any suggestions for adding a unit test?
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG@9
PS2, Line 9: We have a requirement to rewrite the parameters of a function in SQL,
: and there will be more requirements related to SQL parsing in the
: future. Before, we used jsqlParser for parsing. In order to be more
> nit: please wrap each line to have 72 or fewer characters if possible.
Done
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG@25
PS2, Line 25: <artifactId>impala-parser</artifactId>
: <version>${impala.version}</version>
: </dependency>
:
> I prefer using the existing ExprRewriter for customized expr rewrites. Here
I have replaced it with ExprRewriter, which is not universal, so it has not been submitted, and the commit information has been updated.
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml
File fe/pom.xml:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml@1130
PS2, Line 1130: </goals>
> Should we activate this by default?
This profile is generally used by other projects. When building a package, we can use the command: mvn clean package -DskipTests -Pimpala-parser to manually specify. What is the intention to activate by default here?
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/pom.xml@1149
PS2, Line 1149:
> Can we append the impala version at the end? Something like impala-parser-4
Done
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@49
PS2, Line 49:
> why use @SuppressWarnings here?
This method will be deleted.
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS2, Line 82: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipLookupSymbol()) {
> Same question from me. If we do need such a feature, we can add new methods
If we use the createBuiltinWithoutResolvingSymbols method, then we need to add a skip conditional judgment before the createBuiltin method call, which will involve modifications in many places.
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS2, Line 82: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipLookupSymbol()) {
> what is skipLookupSymbol() for?
The createBuiltin method will call the be code through the local method to find the symbol, which requires libfesupport.so when building the parser package. However, it is impossible for the symbol not to be found, so just skip it.
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/StmtVisitor.java
File fe/src/main/java/org/apache/impala/util/StmtVisitor.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/StmtVisitor.java@26
PS2, Line 26:
> Why don't you use or extend org.apache.impala.util.Visitor?
This interface will be deleted.
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 4
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Mon, 05 Dec 2022 07:50:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 2:
(4 comments)
Hi minghui, thanks for your contribution! I post some comments.
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19023/2//COMMIT_MSG@1
PS2, Line 1: Parent: 19114c72 (IMPALA-11578: Exclude locality test for remote FS)
can you add some UT?
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java
File fe/src/main/java/org/apache/impala/analysis/StmtNode.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/analysis/StmtNode.java@49
PS2, Line 49: @SuppressWarnings("unchecked")
why use @SuppressWarnings here?
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
File fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java@82
PS2, Line 82: if (BackendConfig.INSTANCE != null && BackendConfig.INSTANCE.skipLookupSymbol()) {
what is skipLookupSymbol() for?
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/StmtVisitor.java
File fe/src/main/java/org/apache/impala/util/StmtVisitor.java:
http://gerrit.cloudera.org:8080/#/c/19023/2/fe/src/main/java/org/apache/impala/util/StmtVisitor.java@26
PS2, Line 26: List<T> visit(StmtNode node);
Why don't you use or extend org.apache.impala.util.Visitor?
If this interface is necessary, can you add some description javadoc?
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 2
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 30 Sep 2022 02:43:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL,
and there will be more requirements related to SQL parsing in the
future. Before, we used jsqlParser for parsing. In order to be more
compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide
complete parsing capabilities, it is also necessary to support the
toSql method of other statements, such as AlterTableStmt. But in our
scenario, there are only select and use statements, which are already
supported. So all we need to do is to analyze all the functions in sql
and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase stmt = Parser.parse(sql);
ExprRewriter exprRewriter = new MyExprRewritter();
stmt.rewriteExprs(exprRewriter);
return stmt.toSql();
}
The MyExprRewriter class is not a common requirement, so it was not
submitted. This submission is mainly to support the construction of
the parser package.
It should be noted that it must be set in advance:
TBackendGflags tBackendGflags = new TBackendGflags();
tBackendGflags.setSkip_lookup_symbol(true);
BackendConfig.create(tBackendGflags);
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
8 files changed, 73 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/7
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 7
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Minghui Zhu (Code Review)" <ge...@cloudera.org>.
Minghui Zhu has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
We have a requirement to rewrite the parameters of a function in SQL,
and there will be more requirements related to SQL parsing in the
future. Before, we used jsqlParser for parsing. In order to be more
compatible with impala sql, we want to use impala's parser.
Not all statements currently support the toSql method, so to provide
complete parsing capabilities, it is also necessary to support the
toSql method of other statements, such as AlterTableStmt. But in our
scenario, there are only select and use statements, which are already
supported. So all we need to do is to analyze all the functions in sql
and type out the parser package.
We can rely on it in the pom in another project:
<dependency>
<groupId>org.apache.impala</groupId>
<artifactId>impala-parser</artifactId>
<version>${impala.version}</version>
</dependency>
And then we can rewrite the sql:
public String parse(String sql) {
StatementBase stmt = Parser.parse(sql);
ExprRewriter exprRewriter = new MyExprRewritter();
stmt.rewriteExprs(exprRewriter);
return stmt.toSql();
}
The MyExprRewriter class is not a common requirement, so it was not
submitted. This submission is mainly to support the construction of
the parser package.
It should be noted that it must be set in advance:
TBackendGflags tBackendGflags = new TBackendGflags();
tBackendGflags.setSkip_lookup_symbol(true);
BackendConfig.create(tBackendGflags);
Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
---
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/pom.xml
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
8 files changed, 73 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/19023/9
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 9
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12643/ : 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/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 9
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Fri, 17 Mar 2023 03:37:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
Posted by "Xiang Yang (Code Review)" <ge...@cloudera.org>.
Xiang Yang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19023 )
Change subject: IMPALA-11566: Provide SQL parsing capabilities to other applications in the form of jar packages
......................................................................
Patch Set 9:
> Patch Set 9:
>
> Is it capable of parsing multiple SQL statements, such as SELECT 1; SELECT 2?
Hi yi-you, maybe you can split SQL statements by ';', and then pass them to the Parser one by one.
--
To view, visit http://gerrit.cloudera.org:8080/19023
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I246b088310f25b809d9e598c0ecc6613d9749ee3
Gerrit-Change-Number: 19023
Gerrit-PatchSet: 9
Gerrit-Owner: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Anonymous Coward (770)
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jian Zhang <zj...@gmail.com>
Gerrit-Reviewer: John Sherman <jf...@cloudera.com>
Gerrit-Reviewer: Minghui Zhu <hu...@gmail.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Xiang Yang <yx...@126.com>
Gerrit-Comment-Date: Wed, 16 Aug 2023 13:24:00 +0000
Gerrit-HasComments: No