You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "kanha-gupta (via GitHub)" <gi...@apache.org> on 2023/04/30 15:16:00 UTC
[GitHub] [shardingsphere] kanha-gupta opened a new pull request, #25424: Expression queries
kanha-gupta opened a new pull request, #25424:
URL: https://github.com/apache/shardingsphere/pull/25424
Ref #24200
Changes proposed in this pull request:
-
---
Before committing this PR, I'm sure that I have checked the following options:
- [ ] My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [ ] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit tests for my changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183233925
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -474,12 +481,17 @@ private ASTNode createAssignmentSegment(final BooleanPrimaryContext ctx) {
private ASTNode createCompareSegment(final BooleanPrimaryContext ctx) {
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
ExpressionSegment right;
+ String operator;
+ if (null != ctx.ALL()) {
Review Comment:
@strongduanmu Thanks a lot for guidance sir :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183235449
##########
test/it/parser/src/main/resources/sql/supported/dml/select.xml:
##########
@@ -28,8 +28,8 @@
<sql-case id="select_with_same_table_name_and_alias_column_with_owner" value="SELECT t_order.order_id,t_order.user_id,status FROM t_order t_order WHERE t_order.user_id = ? AND order_id = ?" db-types="MySQL,H2" />
<sql-case id="select_not_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id <> ? ORDER BY item_id" />
<sql-case id="select_exclamation_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id != ? ORDER BY item_id" />
- <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" />
- <sql-case id="select_not_between_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT BETWEEN ? AND ? ORDER BY item_id" />
+ <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" db-types="MySQL"/>
Review Comment:
@strongduanmu Sir the reason for changing db-types is because without db-types, the test case is running for other database parsers like SQL92, sqlServer etc. and those files are not optimised for these QUERIES hence Its giving error in InternalSQLParserIT file
I added support for MySQL only. Should I add support for other database parsers too ?
Thank you :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185759659
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
@strongduanmu Okay sir, I will update you with the progress :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185655965
##########
sql-parser/dialect/opengauss/src/main/java/org/apache/shardingsphere/sql/parser/opengauss/visitor/statement/OpenGaussStatementVisitor.java:
##########
@@ -356,7 +356,7 @@ private BinaryOperationExpression createPatternMatchingOperationSegment(final AE
private BinaryOperationExpression createBinaryOperationSegment(final AExprContext ctx, final String operator) {
if ("IS".equalsIgnoreCase(operator)) {
- ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
+ String ifOperator = operator;
Review Comment:
What does ifOperator mean?
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
Can you refer https://github.com/polardb/polardbx-sql/blob/cdd976850bdfc3264dff279198b6b06eb8641fbe/polardbx-optimizer/src/main/java/com/alibaba/polardbx/optimizer/parse/visitor/FastSqlToCalciteNodeVisitor.java#L5346-L5350 to extract `is` and `is not` as an operator, and keep the right to LiteralExpressionSegment?
##########
features/encrypt/core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/condition/EncryptConditionEngine.java:
##########
@@ -77,6 +77,8 @@ public final class EncryptConditionEngine {
SUPPORTED_COMPARE_OPERATOR.add("<=");
SUPPORTED_COMPARE_OPERATOR.add("IS");
Review Comment:
Please remove useless IS operator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185683297
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
Surely sir, I tried it on my local system.
Keeping right to LiteralExpressionSegment is causing double NULL values ->
IS NULL NULL where :
IS NULL -> postfix operator
NULL -> LiteralExpressionSegment
Can you please suggest the fix for this ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183146163
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -474,12 +481,17 @@ private ASTNode createAssignmentSegment(final BooleanPrimaryContext ctx) {
private ASTNode createCompareSegment(final BooleanPrimaryContext ctx) {
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
ExpressionSegment right;
+ String operator;
+ if (null != ctx.ALL()) {
Review Comment:
Hi @kanha-gupta, I test your pr, you can try add `SUPPORTED_COMPARE_OPERATOR.add("IS NULL");
SUPPORTED_COMPARE_OPERATOR.add("IS NOT NULL");` to EncryptConditionEngine, this exception can be solved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185736275
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
@strongduanmu Therefore Please suggest a solution for it.
Thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#issuecomment-1535543921
> @strongduanmu Please review, All database parsers have been optimised now. Thank you
@kanha-gupta Good job, I will review this pr again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183573930
##########
test/it/parser/src/main/resources/sql/supported/dml/select.xml:
##########
@@ -28,8 +28,8 @@
<sql-case id="select_with_same_table_name_and_alias_column_with_owner" value="SELECT t_order.order_id,t_order.user_id,status FROM t_order t_order WHERE t_order.user_id = ? AND order_id = ?" db-types="MySQL,H2" />
<sql-case id="select_not_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id <> ? ORDER BY item_id" />
<sql-case id="select_exclamation_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id != ? ORDER BY item_id" />
- <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" />
- <sql-case id="select_not_between_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT BETWEEN ? AND ? ORDER BY item_id" />
+ <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" db-types="MySQL"/>
Review Comment:
> @strongduanmu Sir the reason for changing db-types is because without db-types, the test case is running for other database parsers like SQL92, sqlServer etc. and those files are not optimised for these QUERIES hence Its giving error in InternalSQLParserIT file I added support for MySQL only. Should I add support for other database parsers too ? Thank you :)
@kanha-gupta, it's better to modify other sql parser logic together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183135965
##########
kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/converter/segment/expression/impl/BinaryOperationExpressionConverter.java:
##########
@@ -86,10 +89,12 @@ public Optional<SqlNode> convert(final BinaryOperationExpression segment) {
private List<SqlNode> convertSqlNodes(final BinaryOperationExpression segment) {
SqlNode left = new ExpressionConverter().convert(segment.getLeft()).orElseThrow(IllegalStateException::new);
- SqlNode right = new ExpressionConverter().convert(segment.getRight()).orElseThrow(IllegalStateException::new);
List<SqlNode> result = new LinkedList<>();
result.add(left);
- result.addAll(right instanceof SqlNodeList ? ((SqlNodeList) right).getList() : Collections.singletonList(right));
+ if (segment.getRight() != null) {
Review Comment:
Please put null condition on the left hand.
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -474,12 +481,17 @@ private ASTNode createAssignmentSegment(final BooleanPrimaryContext ctx) {
private ASTNode createCompareSegment(final BooleanPrimaryContext ctx) {
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
ExpressionSegment right;
+ String operator;
+ if (null != ctx.ALL()) {
Review Comment:
It seems that this parse logic change has impacted encrypt like feature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#issuecomment-1529203465
@strongduanmu Queries in Update.xml test file is giving the above error while running EncryptSQLRewriterIT.java file.
can you please suggest whether changes are required in XML Files or In Code files ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1184535816
##########
test/it/parser/src/main/resources/sql/supported/dml/select.xml:
##########
@@ -28,8 +28,8 @@
<sql-case id="select_with_same_table_name_and_alias_column_with_owner" value="SELECT t_order.order_id,t_order.user_id,status FROM t_order t_order WHERE t_order.user_id = ? AND order_id = ?" db-types="MySQL,H2" />
<sql-case id="select_not_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id <> ? ORDER BY item_id" />
<sql-case id="select_exclamation_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id != ? ORDER BY item_id" />
- <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" />
- <sql-case id="select_not_between_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT BETWEEN ? AND ? ORDER BY item_id" />
+ <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" db-types="MySQL"/>
Review Comment:
okay
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta closed pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta closed pull request #25424: Support for boolean primary MySQL queries
URL: https://github.com/apache/shardingsphere/pull/25424
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183235449
##########
test/it/parser/src/main/resources/sql/supported/dml/select.xml:
##########
@@ -28,8 +28,8 @@
<sql-case id="select_with_same_table_name_and_alias_column_with_owner" value="SELECT t_order.order_id,t_order.user_id,status FROM t_order t_order WHERE t_order.user_id = ? AND order_id = ?" db-types="MySQL,H2" />
<sql-case id="select_not_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id <> ? ORDER BY item_id" />
<sql-case id="select_exclamation_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id != ? ORDER BY item_id" />
- <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" />
- <sql-case id="select_not_between_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT BETWEEN ? AND ? ORDER BY item_id" />
+ <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" db-types="MySQL"/>
Review Comment:
@strongduanmu Sir the reason for changing db-types is because without db-types, the test case is running for other database parsers like SQL92, sqlServer etc. and those files are not optimised for these QUERIES.
I added support for MySQL only. Should I add support for other database parsers too ?
Thank you :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185733719
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
@strongduanmu Sir, I tried it. The problem I am encountering is that Calcite's SqlStdOperatorTable do not have "IS" as a function or PostFixOperator. Therefore While breaking "IS NULL" into "IS" as operator & "NULL" in right as LiteralExpressionSegment, It gives error "IS" is not supported.
Same problem is for "IS NOT NULL" as SqlStdOperatorTable doesnt support "IS NOT"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1187010644
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
@strongduanmu Sir, should I just revise CodeStyle ? in the example code, operator is set when Right is instanceof SqlNullExpr. Therefore I can understand that the Logic would stay same just like this current PR ?
Thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183141597
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -474,12 +481,17 @@ private ASTNode createAssignmentSegment(final BooleanPrimaryContext ctx) {
private ASTNode createCompareSegment(final BooleanPrimaryContext ctx) {
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
ExpressionSegment right;
+ String operator;
+ if (null != ctx.ALL()) {
Review Comment:
It seems that this parse logic change has impacted encrypt is null feature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#issuecomment-1534730829
@strongduanmu Please review, All database parsers have been optimised now.
Thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185686099
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
You can modify is null operator to is operator, is not null operator to is not operator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185737737
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -441,7 +446,9 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
ExpressionSegment right = new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 2, ctx.stop.getStopIndex(), rightText);
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
- String operator = "IS";
+ if (null != ctx.NULL()) {
+ right = null;
Review Comment:
> Can you refer https://github.com/polardb/polardbx-sql/blob/cdd976850bdfc3264dff279198b6b06eb8641fbe/polardbx-optimizer/src/main/java/com/alibaba/polardbx/optimizer/parse/visitor/FastSqlToCalciteNodeVisitor.java#L5346-L5350 to extract `is` and `is not` as an operator, and keep the right to LiteralExpressionSegment?
@kanha-gupta Can you take a look at this link? It provide an example to convert to sql node.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#issuecomment-1529077821
@strongduanmu Respected sir, This PR works well (SQLNodeConverterEngineIT and XML tests are successful) but I am getting a maven error :
Error: Tests run: 265, Failures: 0, Errors: 8, Skipped: 0, Time elapsed: 6.686 s <<< FAILURE! - in org.apache.shardingsphere.test.it.rewrite.engine.scenario.EncryptSQLRewriterIT
Error: assertRewrite{SQLRewriteEngineTestParameters}[145] Time elapsed: 0.022 s <<< ERROR!
org.apache.shardingsphere.encrypt.exception.syntax.UnsupportedEncryptSQLException: The SQL clause `IS NULL` is unsupported in encrypt rule.
what is encrypt rule & how to add support for 'IS NULL' ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183146494
##########
test/it/parser/src/main/resources/sql/supported/dml/select.xml:
##########
@@ -28,8 +28,8 @@
<sql-case id="select_with_same_table_name_and_alias_column_with_owner" value="SELECT t_order.order_id,t_order.user_id,status FROM t_order t_order WHERE t_order.user_id = ? AND order_id = ?" db-types="MySQL,H2" />
<sql-case id="select_not_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id <> ? ORDER BY item_id" />
<sql-case id="select_exclamation_equal_with_single_table" value="SELECT * FROM t_order_item WHERE item_id != ? ORDER BY item_id" />
- <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" />
- <sql-case id="select_not_between_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT BETWEEN ? AND ? ORDER BY item_id" />
+ <sql-case id="select_not_in_with_single_table" value="SELECT * FROM t_order_item WHERE item_id IS NOT NULL AND item_id NOT IN (?, ?) ORDER BY item_id" db-types="MySQL"/>
Review Comment:
Please do not change db-types here. This will reduce our test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1183233925
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -474,12 +481,17 @@ private ASTNode createAssignmentSegment(final BooleanPrimaryContext ctx) {
private ASTNode createCompareSegment(final BooleanPrimaryContext ctx) {
ExpressionSegment left = (ExpressionSegment) visit(ctx.booleanPrimary());
ExpressionSegment right;
+ String operator;
+ if (null != ctx.ALL()) {
Review Comment:
Thanks a lot for guidance sir :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185685199
##########
sql-parser/dialect/opengauss/src/main/java/org/apache/shardingsphere/sql/parser/opengauss/visitor/statement/OpenGaussStatementVisitor.java:
##########
@@ -356,7 +356,7 @@ private BinaryOperationExpression createPatternMatchingOperationSegment(final AE
private BinaryOperationExpression createBinaryOperationSegment(final AExprContext ctx, final String operator) {
if ("IS".equalsIgnoreCase(operator)) {
- ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
+ String ifOperator = operator;
Review Comment:
Sir, I'll rename it. The variable helps in storing & changing String values when IF conditions are met.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] kanha-gupta commented on a diff in pull request #25424: Support for boolean primary MySQL queries
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #25424:
URL: https://github.com/apache/shardingsphere/pull/25424#discussion_r1185685199
##########
sql-parser/dialect/opengauss/src/main/java/org/apache/shardingsphere/sql/parser/opengauss/visitor/statement/OpenGaussStatementVisitor.java:
##########
@@ -356,7 +356,7 @@ private BinaryOperationExpression createPatternMatchingOperationSegment(final AE
private BinaryOperationExpression createBinaryOperationSegment(final AExprContext ctx, final String operator) {
if ("IS".equalsIgnoreCase(operator)) {
- ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
+ String ifOperator = operator;
Review Comment:
Sir, I'll rename it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org