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/06/03 20:12:34 UTC
[GitHub] [shardingsphere] kanha-gupta opened a new pull request, #26038: support for WindowSegment
kanha-gupta opened a new pull request, #26038:
URL: https://github.com/apache/shardingsphere/pull/26038
Fixes #ISSUSE_ID.
Changes proposed in this pull request:
1. Adds support for window segment
2. there is a constructor added in WindowSegment.java file to prevent errors caused by File usages
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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#discussion_r1224581966
##########
parser/sql/dialect/postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/statement/PostgreSQLStatementVisitor.java:
##########
@@ -980,9 +982,22 @@ public ASTNode visitHavingClause(final HavingClauseContext ctx) {
@Override
public ASTNode visitWindowClause(final WindowClauseContext ctx) {
+ if (null != ctx.windowDefinitionList()) {
+ return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getWindowItem(ctx.windowDefinitionList().windowDefinition()),
+ getWindowSpecification(ctx.windowDefinitionList().windowDefinition().windowSpecification()));
+ }
return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex());
}
+ private IdentifierValue getWindowItem(final WindowDefinitionContext ctx) {
+ return new IdentifierValue(ctx.colId().identifier().getText());
+ }
+
+ private Collection<ExpressionSegment> getWindowSpecification(final WindowSpecificationContext ctx) {
Review Comment:
sir, I have added sql parse test case in the new xml file. is there any other test to add ?
--
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] boyjoy1127 commented on pull request #26038: support for WindowSegment
Posted by "boyjoy1127 (via GitHub)" <gi...@apache.org>.
boyjoy1127 commented on PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#issuecomment-1585976921
Will this issue be completed before June 15 which is the final date of version 5.4.0?
--
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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#issuecomment-1583055028
@strongduanmu @RaigorJiang Please review.
Conflict and CI errors are resolved now :)
--
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 #26038: support for WindowSegment
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#discussion_r1223862524
##########
parser/sql/dialect/mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/statement/MySQLStatementVisitor.java:
##########
@@ -768,9 +770,25 @@ public ASTNode visitTableStatement(final TableStatementContext ctx) {
@Override
public ASTNode visitWindowClause(final WindowClauseContext ctx) {
+ if (null != ctx.windowItem()) {
+ return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getWindowItem(ctx.windowItem(0)),
+ getWindowSpecification(ctx.windowItem(0).windowSpecification()));
+ }
return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex());
}
+ private IdentifierValue getWindowItem(final WindowItemContext ctx) {
+ return new IdentifierValue(ctx.identifier().getText());
+ }
+
+ private Collection<ExpressionSegment> getWindowSpecification(final WindowSpecificationContext ctx) {
+ Collection<ExpressionSegment> segments = new LinkedList<>();
+ for (ExprContext each : ctx.expr()) {
+ segments.add((ExpressionSegment) visit(each));
+ }
+ return segments;
Review Comment:
Please rename segments to result.
##########
parser/sql/dialect/postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/statement/PostgreSQLStatementVisitor.java:
##########
@@ -980,9 +982,22 @@ public ASTNode visitHavingClause(final HavingClauseContext ctx) {
@Override
public ASTNode visitWindowClause(final WindowClauseContext ctx) {
+ if (null != ctx.windowDefinitionList()) {
+ return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getWindowItem(ctx.windowDefinitionList().windowDefinition()),
+ getWindowSpecification(ctx.windowDefinitionList().windowDefinition().windowSpecification()));
+ }
return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex());
}
+ private IdentifierValue getWindowItem(final WindowDefinitionContext ctx) {
+ return new IdentifierValue(ctx.colId().identifier().getText());
+ }
+
+ private Collection<ExpressionSegment> getWindowSpecification(final WindowSpecificationContext ctx) {
Review Comment:
Hi @kanha-gupta, can you add sql parse test case for new visit logic?
--
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 #26038: support for WindowSegment
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu commented on code in PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#discussion_r1225234363
##########
parser/sql/dialect/opengauss/src/main/java/org/apache/shardingsphere/sql/parser/opengauss/visitor/statement/OpenGaussStatementVisitor.java:
##########
@@ -1016,9 +1018,22 @@ public ASTNode visitHavingClause(final HavingClauseContext ctx) {
@Override
public ASTNode visitWindowClause(final WindowClauseContext ctx) {
+ if (null != ctx.windowDefinitionList()) {
+ return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getWindowItem(ctx.windowDefinitionList().windowDefinition()),
+ getWindowSpecification(ctx.windowDefinitionList().windowDefinition().windowSpecification()));
+ }
return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex());
}
+ private IdentifierValue getWindowItem(final WindowDefinitionContext ctx) {
+ return new IdentifierValue(ctx.colId().identifier().getText());
+ }
+
+ private Collection<ExpressionSegment> getWindowSpecification(final WindowSpecificationContext ctx) {
+ Collection<ExpressionSegment> result = createInsertValuesSegments(ctx.partitionClause().exprList());
Review Comment:
Hi @kanha-gupta, why call createInsertValuesSegments method in getWindowSpecification?
##########
parser/sql/statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/common/segment/generic/WindowSegment.java:
##########
@@ -31,4 +35,15 @@ public final class WindowSegment implements SQLSegment {
private final int startIndex;
private final int stopIndex;
+
+ private IdentifierValue identifierValue;
+
+ private Collection<ExpressionSegment> segments;
+
+ public WindowSegment(final int startIndex, final int stopIndex, final IdentifierValue identifierValue, final Collection<ExpressionSegment> segments) {
+ this.startIndex = startIndex;
+ this.stopIndex = stopIndex;
+ this.identifierValue = identifierValue;
+ this.segments = segments;
Review Comment:
Can you rename it to more meaningful name?
--
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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#issuecomment-1586561964
> @kanha-gupta Thank you for your outstanding work.
Thanks a lot 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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#discussion_r1225877893
##########
parser/sql/statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/common/segment/generic/WindowSegment.java:
##########
@@ -31,4 +35,15 @@ public final class WindowSegment implements SQLSegment {
private final int startIndex;
private final int stopIndex;
+
+ private IdentifierValue identifierValue;
+
+ private Collection<ExpressionSegment> segments;
+
+ public WindowSegment(final int startIndex, final int stopIndex, final IdentifierValue identifierValue, final Collection<ExpressionSegment> segments) {
+ this.startIndex = startIndex;
+ this.stopIndex = stopIndex;
+ this.identifierValue = identifierValue;
+ this.segments = segments;
Review Comment:
done
--
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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#issuecomment-1584938934
@strongduanmu Please review again, changes are made :)
--
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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#discussion_r1225237010
##########
parser/sql/dialect/opengauss/src/main/java/org/apache/shardingsphere/sql/parser/opengauss/visitor/statement/OpenGaussStatementVisitor.java:
##########
@@ -1016,9 +1018,22 @@ public ASTNode visitHavingClause(final HavingClauseContext ctx) {
@Override
public ASTNode visitWindowClause(final WindowClauseContext ctx) {
+ if (null != ctx.windowDefinitionList()) {
+ return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getWindowItem(ctx.windowDefinitionList().windowDefinition()),
+ getWindowSpecification(ctx.windowDefinitionList().windowDefinition().windowSpecification()));
+ }
return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex());
}
+ private IdentifierValue getWindowItem(final WindowDefinitionContext ctx) {
+ return new IdentifierValue(ctx.colId().identifier().getText());
+ }
+
+ private Collection<ExpressionSegment> getWindowSpecification(final WindowSpecificationContext ctx) {
+ Collection<ExpressionSegment> result = createInsertValuesSegments(ctx.partitionClause().exprList());
Review Comment:
Sir, thank you for the review :) createInsertValuesSegments method Gives desired Result for exprList() processing therefore I call that method instead of writing code for it.
Is the approach good or Should I write new code 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] RaigorJiang commented on pull request #26038: support for WindowSegment
Posted by "RaigorJiang (via GitHub)" <gi...@apache.org>.
RaigorJiang commented on PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#issuecomment-1579742591
Hi @kanha-gupta
Please fix the file conflict, 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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on code in PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#discussion_r1224581966
##########
parser/sql/dialect/postgresql/src/main/java/org/apache/shardingsphere/sql/parser/postgresql/visitor/statement/PostgreSQLStatementVisitor.java:
##########
@@ -980,9 +982,22 @@ public ASTNode visitHavingClause(final HavingClauseContext ctx) {
@Override
public ASTNode visitWindowClause(final WindowClauseContext ctx) {
+ if (null != ctx.windowDefinitionList()) {
+ return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), getWindowItem(ctx.windowDefinitionList().windowDefinition()),
+ getWindowSpecification(ctx.windowDefinitionList().windowDefinition().windowSpecification()));
+ }
return new WindowSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex());
}
+ private IdentifierValue getWindowItem(final WindowDefinitionContext ctx) {
+ return new IdentifierValue(ctx.colId().identifier().getText());
+ }
+
+ private Collection<ExpressionSegment> getWindowSpecification(final WindowSpecificationContext ctx) {
Review Comment:
sir, I have added sql parse test case in the new xml file for Mysql, PostgreSQL & opengauss.
is there any other test to add ?
--
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 #26038: support for WindowSegment
Posted by "kanha-gupta (via GitHub)" <gi...@apache.org>.
kanha-gupta commented on PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038#issuecomment-1575177664
@strongduanmu Please review, is the approach good ?
this caseID is also for PostgreSQL & OpenGauss which I will optimise after you approve the Approach and code.
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 #26038: support for WindowSegment
Posted by "strongduanmu (via GitHub)" <gi...@apache.org>.
strongduanmu merged PR #26038:
URL: https://github.com/apache/shardingsphere/pull/26038
--
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