Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for boolean primary MySQL queries #25424

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public final class EncryptConditionEngine {
SUPPORTED_COMPARE_OPERATOR.add("<=");
SUPPORTED_COMPARE_OPERATOR.add("IS");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove useless IS operator.

SUPPORTED_COMPARE_OPERATOR.add("LIKE");
SUPPORTED_COMPARE_OPERATOR.add("IS NULL");
SUPPORTED_COMPARE_OPERATOR.add("IS NOT NULL");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ private static void register() {
register(SqlStdOperatorTable.LIKE);
register(SqlStdOperatorTable.NOT_LIKE);
register(SqlStdOperatorTable.PERCENT_REMAINDER);
register(SqlStdOperatorTable.IS_NULL);
register(SqlStdOperatorTable.IS_NOT_NULL);
register(SqlStdOperatorTable.ALL_GT);
}

private static void register(final SqlOperator sqlOperator) {
Expand All @@ -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 (null != segment.getRight()) {
SqlNode right = new ExpressionConverter().convert(segment.getRight()).orElseThrow(IllegalStateException::new);
result.addAll(right instanceof SqlNodeList ? ((SqlNodeList) right).getList() : Collections.singletonList(right));
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ private BinaryOperationExpression createBinaryOperationExpression(final ExprCont
public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
if (null != ctx.IS()) {
// TODO optimize operatorToken
String operator = "IS";
String rightText = "";
if (null != ctx.NOT()) {
rightText = rightText.concat(ctx.start.getInputStream().getText(new Interval(ctx.NOT().getSymbol().getStartIndex(),
Expand All @@ -429,6 +430,10 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
Token operatorToken = null;
if (null != ctx.NULL()) {
operatorToken = ctx.NULL().getSymbol();
operator = "IS NULL";
}
if (null != ctx.NULL() && null != ctx.NOT()) {
operator = "IS NOT NULL";
}
if (null != ctx.TRUE()) {
operatorToken = ctx.TRUE().getSymbol();
Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can modify is null operator to is operator, is not null operator to is not operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strongduanmu Therefore Please suggest a solution for it.
Thank you

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strongduanmu Okay sir, I will update you with the progress :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

}
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, operator, text);
}
if (null != ctx.comparisonOperator() || null != ctx.SAFE_EQ_()) {
Expand Down Expand Up @@ -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()) {
operator = null != ctx.SAFE_EQ_() ? ctx.SAFE_EQ_().getText() : ctx.comparisonOperator().getText() + " ALL";
} else {
operator = null != ctx.SAFE_EQ_() ? ctx.SAFE_EQ_().getText() : ctx.comparisonOperator().getText();
}
if (null != ctx.predicate()) {
right = (ExpressionSegment) visit(ctx.predicate());
} else {
right = new SubqueryExpressionSegment(new SubquerySegment(ctx.subquery().start.getStartIndex(), ctx.subquery().stop.getStopIndex(), (MySQLSelectStatement) visit(ctx.subquery())));
}
String operator = null != ctx.SAFE_EQ_() ? ctx.SAFE_EQ_().getText() : ctx.comparisonOperator().getText();
String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, operator, text);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does ifOperator mean?

Copy link
Contributor Author

@kanha-gupta kanha-gupta May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sir, I'll rename it. The variable helps in storing & changing String values when IF conditions are met.

String rightText;
ExpressionSegment right;
if (null != ctx.IS()) {
Expand All @@ -366,7 +366,15 @@ private BinaryOperationExpression createBinaryOperationSegment(final AExprContex
rightText = ctx.start.getInputStream().getText(new Interval(ctx.ISNULL().getSymbol().getStartIndex() + 2, ctx.stop.getStopIndex())).trim();
right = new LiteralExpressionSegment(ctx.ISNULL().getSymbol().getStartIndex() + 2, ctx.stop.getStopIndex(), rightText);
}
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, "IS",
if (null != ctx.NULL()) {
ifOperator = "IS NULL";
right = null;
}
if (null != ctx.NULL() && null != ctx.NOT()) {
ifOperator = "IS NOT NULL";
}
ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, ifOperator,
ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex())));
}
ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,15 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
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.IS()) {
if (null != ctx.NULL()) {
operator = "IS NULL";
right = null;
}
if (null != ctx.NULL() && null != ctx.NOT()) {
operator = "IS NOT NULL";
}
}
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, operator, text);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
String rightText;
ExpressionSegment right;
if (null != ctx.IS()) {
Expand All @@ -366,7 +366,15 @@ private BinaryOperationExpression createBinaryOperationSegment(final AExprContex
rightText = ctx.start.getInputStream().getText(new Interval(ctx.ISNULL().getSymbol().getStartIndex() + 2, ctx.stop.getStopIndex())).trim();
right = new LiteralExpressionSegment(ctx.ISNULL().getSymbol().getStartIndex() + 2, ctx.stop.getStopIndex(), rightText);
}
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, "IS",
if (null != ctx.NULL()) {
ifOperator = "IS NULL";
right = null;
}
if (null != ctx.NULL() && null != ctx.NOT()) {
ifOperator = "IS NOT NULL";
}
ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, ifOperator,
ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex())));
}
ExpressionSegment left = (ExpressionSegment) visit(ctx.aExpr(0));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ private ASTNode createBinaryOperationExpression(final ExprContext ctx, final Str
@Override
public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
if (null != ctx.IS()) {
String operator = "IS";
String rightText = "";
if (null != ctx.NOT()) {
rightText = rightText.concat(ctx.start.getInputStream().getText(new Interval(ctx.NOT().getSymbol().getStartIndex(),
Expand All @@ -261,6 +262,10 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
Token operatorToken = null;
if (null != ctx.NULL()) {
operatorToken = ctx.NULL().getSymbol();
operator = "IS NULL";
}
if (null != ctx.NULL() && null != ctx.NOT()) {
operator = "IS NOT NULL";
}
if (null != ctx.TRUE()) {
operatorToken = ctx.TRUE().getSymbol();
Expand All @@ -273,7 +278,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;
}
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, operator, text);
}
if (null != ctx.comparisonOperator() || null != ctx.SAFE_EQ_()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ private ASTNode createBinaryOperationExpression(final ExprContext ctx, final Str
@Override
public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
if (null != ctx.IS()) {
String operator = "IS";
String rightText = "";
if (null != ctx.NOT()) {
rightText = rightText.concat(ctx.start.getInputStream().getText(new Interval(ctx.NOT().getSymbol().getStartIndex(),
Expand All @@ -376,6 +377,10 @@ public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
Token operatorToken = null;
if (null != ctx.NULL()) {
operatorToken = ctx.NULL().getSymbol();
operator = "IS NULL";
}
if (null != ctx.NULL() && null != ctx.NOT()) {
operator = "IS NOT NULL";
}
if (null != ctx.TRUE()) {
operatorToken = ctx.TRUE().getSymbol();
Expand All @@ -388,7 +393,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;
}
return new BinaryOperationExpression(ctx.start.getStartIndex(), ctx.stop.getStopIndex(), left, right, operator, text);
}
if (null != ctx.comparisonOperator() || null != ctx.SAFE_EQ_()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ private Collection<String> getSupportedSQLCaseIDs() {
result.add("select_substring");
result.add("select_where_with_bit_expr_with_plus_interval");
result.add("select_where_with_bit_expr_with_minus_interval");
result.add("select_where_with_boolean_primary_with_is");
result.add("select_where_with_boolean_primary_with_is_not");
result.add("select_where_with_boolean_primary_with_comparison_subquery");
return result;
}
// CHECKSTYLE:ON
Expand Down
12 changes: 3 additions & 9 deletions test/it/parser/src/main/resources/case/dml/select-expression.xml
Original file line number Diff line number Diff line change
Expand Up @@ -643,10 +643,7 @@
<owner name="t_order" start-index="28" stop-index="34" />
</column>
</left>
<operator>IS</operator>
<right>
<literal-expression value="NULL" start-index="46" stop-index="49" />
</right>
<operator>IS NULL</operator>
</binary-operation-expression>
</expr>
</where>
Expand All @@ -667,10 +664,7 @@
<owner name="t_order" start-index="28" stop-index="34" />
</column>
</left>
<operator>IS</operator>
<right>
<literal-expression value="NOT NULL" start-index="46" stop-index="53" />
</right>
<operator>IS NOT NULL</operator>
</binary-operation-expression>
</expr>
</where>
Expand Down Expand Up @@ -743,7 +737,7 @@
<owner name="t_order" start-index="28" stop-index="34" />
</column>
</left>
<operator>&gt;</operator>
<operator>> ALL</operator>
<right>
<subquery start-index="49" stop-index="98">
<select>
Expand Down
10 changes: 2 additions & 8 deletions test/it/parser/src/main/resources/case/dml/select-with.xml
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,7 @@
<left>
<column name="manager_id" start-index="164" stop-index="173" />
</left>
<operator>IS</operator>
<right>
<literal-expression value="NULL" start-index="178" stop-index="181" />
</right>
<operator>IS NULL</operator>
</binary-operation-expression>
</expr>
</where>
Expand Down Expand Up @@ -219,10 +216,7 @@
<left>
<column name="manager_id" start-index="173" stop-index="182" />
</left>
<operator>IS</operator>
<right>
<literal-expression value="NULL" start-index="187" stop-index="190" />
</right>
<operator>IS NULL</operator>
</binary-operation-expression>
</expr>
</where>
Expand Down
11 changes: 2 additions & 9 deletions test/it/parser/src/main/resources/case/dml/select.xml
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,7 @@
<left>
<column name="item_id" start-index="33" stop-index="39" />
</left>
<operator>IS</operator>
<right>
<literal-expression value="NOT NULL" start-index="44" stop-index="51" />
</right>
<operator>IS NOT NULL</operator>
</binary-operation-expression>
<common-expression text="item_id IS NOT NULL" start-index="33" stop-index="51" />
</left>
Expand Down Expand Up @@ -336,11 +333,7 @@
<left>
<column name="item_id" start-index="33" stop-index="39" />
</left>
<operator>IS</operator>
<right>
<!-- TODO: literal value should be 'NOT NULL' -->
<literal-expression value="NOT NULL" start-index="44" stop-index="51" />
</right>
<operator>IS NOT NULL</operator>
</binary-operation-expression>
<common-expression text="item_id IS NOT NULL" start-index="33" stop-index="51" />
</left>
Expand Down