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/03/29 04:18:35 UTC
[GitHub] [shardingsphere] kanha-gupta opened a new pull request, #24888: support for Char function
kanha-gupta opened a new pull request, #24888:
URL: https://github.com/apache/shardingsphere/pull/24888
Fixes #ISSUSE_ID.
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] strongduanmu commented on a diff in pull request #24888: support for Char function
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#discussion_r1153423599
##########
test/it/parser/src/main/resources/case/dml/select-special-function.xml:
##########
@@ -125,10 +125,26 @@
</projections>
</select>
<select sql-case-id="select_char">
- <projections start-index="7" stop-index="29">
- <expression-projection text="CHAR(77,121,83,81,'76')" start-index="7" stop-index="29">
+ <projections start-index="7" stop-index="44">
Review Comment:
> I optimised it. Thanks alot :)
That's great. I will review this pr again after ci passed.
--
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 #24888: support for Char function
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#discussion_r1152766465
##########
test/it/parser/src/main/resources/case/dml/select-special-function.xml:
##########
@@ -125,10 +125,26 @@
</projections>
</select>
<select sql-case-id="select_char">
- <projections start-index="7" stop-index="29">
- <expression-projection text="CHAR(77,121,83,81,'76')" start-index="7" stop-index="29">
+ <projections start-index="7" stop-index="44">
Review Comment:
Why modify `stop-index` to `44`? The projection stop index in sql `SELECT CHAR(77,121,83,81,'76')` is 29, you should keep it with correct value.
##########
test/it/parser/src/main/resources/case/dml/select-special-function.xml:
##########
@@ -125,10 +125,26 @@
</projections>
</select>
<select sql-case-id="select_char">
- <projections start-index="7" stop-index="29">
- <expression-projection text="CHAR(77,121,83,81,'76')" start-index="7" stop-index="29">
+ <projections start-index="7" stop-index="44">
+ <expression-projection text="CHAR(77,121,83,81,'76')" start-index="7" stop-index="44">
<expr>
- <function function-name="CHAR" start-index="7" stop-index="29" text="CHAR(77,121,83,81,'76')" />
+ <function function-name="CHAR" start-index="7" stop-index="44" text="CHAR(77,121,83,81,'76')" >
+ <parameter>
+ <literal-expression value="77" start-index="12" stop-index="13" />
+ </parameter>
+ <parameter>
+ <literal-expression value="121" start-index="15" stop-index="17" />
+ </parameter>
+ <parameter>
+ <literal-expression value="83" start-index="20" stop-index="21" />
+ </parameter>
+ <parameter>
+ <literal-expression value="81" start-index="24" stop-index="25" />
+ </parameter>
+ <parameter>
+ <literal-expression value="'76'" start-index="29" stop-index="32" />
Review Comment:
This start-index and stop-index seems also wrong.
--
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 #24888: support for Char function
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#issuecomment-1487925753
@strongduanmu Hello, I request you to review it please.
task #1
--
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 #24888: support for SELECT special functions
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#issuecomment-1493539795
Hi @kanha-gupta, can you merge master branch?
--
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 #24888: support for SELECT special functions
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#discussion_r1156924684
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLStatementSQLVisitor.java:
##########
@@ -954,7 +954,12 @@ public final ASTNode visitExtractFunction(final ExtractFunctionContext ctx) {
@Override
public final ASTNode visitCharFunction(final CharFunctionContext ctx) {
calculateParameterCount(ctx.expr());
- return new FunctionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ctx.CHAR().getText(), getOriginalText(ctx));
+ FunctionSegment result = new FunctionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ctx.CHAR().getText(), getOriginalText(ctx));
+ for (ExprContext each : ctx.expr()) {
+ ASTNode expr = visit(each);
+ result.getParameters().add((LiteralExpressionSegment) expr);
Review Comment:
Do you think use `(ExpressionSegment) visit(ctx.expr())` is better? `(LiteralExpressionSegment) visit(ctx.expr())` will cause class cast exception.
##########
sql-parser/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/impl/MySQLStatementSQLVisitor.java:
##########
@@ -966,7 +971,9 @@ public final ASTNode visitTrimFunction(final TrimFunctionContext ctx) {
@Override
public final ASTNode visitWeightStringFunction(final WeightStringFunctionContext ctx) {
calculateParameterCount(Collections.singleton(ctx.expr()));
- return new FunctionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ctx.WEIGHT_STRING().getText(), getOriginalText(ctx));
+ FunctionSegment result = new FunctionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), ctx.WEIGHT_STRING().getText(), getOriginalText(ctx));
+ result.getParameters().add((LiteralExpressionSegment) visit(ctx.expr()));
Review Comment:
Do you think use `(ExpressionSegment) visit(ctx.expr())` is better? `(LiteralExpressionSegment) visit(ctx.expr())` will cause class cast exception.
--
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 #24888: support for Char function
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#issuecomment-1487941094
Hi @kanha-gupta, can you refer issue #24200 for this pr?
--
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 #24888: support for Char function
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#issuecomment-1489673848
@strongduanmu Hello
The code is getting build error Failures:
[ERROR] InternalMySQLParserIT>InternalSQLParserIT.assertSupportedSQL:60
SQL Case ID : select_char
SQL : SELECT CHAR(77,121,83,81,'76')
`ProjectionsSegment`'s stop index assertion error:
Expected: is <44>
but: was <29>
XML code already have expected value at stop index & therefore I couldnt reach to solution.
Can you please suggest error resolution ?
Thank you
(select_char test is being passed successfully in SQLNodeConverterEngineIT.java)
--
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 #24888: support for Char function
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#discussion_r1153258939
##########
test/it/parser/src/main/resources/case/dml/select-special-function.xml:
##########
@@ -125,10 +125,26 @@
</projections>
</select>
<select sql-case-id="select_char">
- <projections start-index="7" stop-index="29">
- <expression-projection text="CHAR(77,121,83,81,'76')" start-index="7" stop-index="29">
+ <projections start-index="7" stop-index="44">
Review Comment:
I optimised it. Thanks alot :)
--
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 #24888: support for Char function
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#issuecomment-1492878420
@strongduanmu Hello,
Can you please suggest How to parse Identifiers & symbols in MySQLStatementSQLVisitor.java & related XML files from their Antlr grammar files ?
I tried finding a solution but I cant come up to solution looking at existing codebase.
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 merged pull request #24888: support for SELECT special functions
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu merged PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888
--
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 #24888: support for SELECT special functions
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #24888:
URL: https://github.com/apache/shardingsphere/pull/24888#issuecomment-1493626535
@strongduanmu Done, please check.
--
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