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

sql parser support clickHouse #31226

Merged
merged 4 commits into from
May 16, 2024
Merged

Conversation

zzyReal666
Copy link
Contributor

Fixes #31224.

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 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.
    image

Copy link
Member

@linghengqian linghengqian left a comment

Choose a reason for hiding this comment

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

  • The PR did not pass the code formatting validation. You can execute first ./mvnw spotless:apply -Pcheck -T1C. Then execute ./mvnw checkstyle:check -Pcheck -T1C to manually adjust to repair the CI.

@zzyReal666
Copy link
Contributor Author

  • The PR did not pass the code formatting validation. You can execute first ./mvnw spotless:apply -Pcheck -T1C. Then execute ./mvnw checkstyle:check -Pcheck -T1C to manually adjust to repair the CI.

Got it!

linghengqian
linghengqian previously approved these changes May 15, 2024
Copy link
Member

@linghengqian linghengqian left a comment

Choose a reason for hiding this comment

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

  • LGTM. Just waiting for further review by other committers.


unreservedWord
: AFTER | ALIAS | ALL | ALTER | AND | ANTI | ANY | ARRAY | AS | ASCENDING | ASOF | AST | ASYNC | ATTACH | BETWEEN | BOTH | BY | CASE
| CAST | CHECK | CLEAR | CLUSTER | CODEC | COLLATE | COLUMN | COMMENT | CONSTRAINT | CREATE | CROSS | CUBE | CURRENT | DATABASE
Copy link
Member

Choose a reason for hiding this comment

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

Please keep same indent with previous line.

| TABLES | TEMPORARY | TEST | THEN | TIES | TIMEOUT | TIMESTAMP | TOTALS | TRAILING | TRIM | TRUNCATE | TO | TOP | TTL | TYPE
| UNBOUNDED | UNION | UPDATE | USE | USING | UUID | VALUES | VIEW | VOLUME | WATCH | WHEN | WHERE | WINDOW | WITH
;
interval: SECOND | MINUTE | HOUR | DAY | WEEK | MONTH | QUARTER | YEAR;
Copy link
Member

Choose a reason for hiding this comment

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

Please add new line before interval.

grammar BaseRule;

import LexerSymbol, LexerKeyword, LexerClickHouseKeyword, LexerLiterals;

Copy link
Member

Choose a reason for hiding this comment

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

Please only keep one line here.


delete
:
// DELETE singleTableClause whereClause?
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 sytax.

/**
* Statement visitor facade for ClickHouse.
*/
public class ClickHouseStatementVisitorFacade implements SQLStatementVisitorFacade {
Copy link
Member

Choose a reason for hiding this comment

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

Please add final for this class.

/**
* ClickHouse DAL statement visitor.
*/
public class ClickHouseDALStatementVisitor extends ClickHouseStatementVisitor implements DALStatementVisitor {
Copy link
Member

Choose a reason for hiding this comment

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

Please add final for this class.

/**
* ClickHouse DCL statement visitor.
*/
public class ClickHouseDCLStatementVisitor extends ClickHouseStatementVisitor implements DCLStatementVisitor {
Copy link
Member

Choose a reason for hiding this comment

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

Please add final for this class.

/**
* ClickHouse DDL statement visitor.
*/
public class ClickHouseDDLStatementVisitor extends ClickHouseStatementVisitor implements DDLStatementVisitor {
Copy link
Member

Choose a reason for hiding this comment

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

Please add final for this class.

/**
* ClickHouse RL statement visitor.
*/
public class ClickHouseRLStatementVisitor extends ClickHouseStatementVisitor implements RLStatementVisitor {
Copy link
Member

Choose a reason for hiding this comment

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

Please add final for this class.

/**
* ClickHouse TCL statement visitor.
*/
public class ClickHouseTCLStatementVisitor extends ClickHouseStatementVisitor implements TCLStatementVisitor {
Copy link
Member

Choose a reason for hiding this comment

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

Please add final for this class.

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 All done.

update g4 file to keep format consistent;
Copy link
Member

@strongduanmu strongduanmu left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @zzyReal666

@strongduanmu strongduanmu merged commit 29166be into apache:master May 16, 2024
143 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQL parser support ClickHouse
3 participants