Skip to content

Conversation

@Lagovas
Copy link
Collaborator

@Lagovas Lagovas commented Nov 22, 2022

Extended parser with postgresql specific statements syntax and refactored rendering from the AST to the string in same format as it was parsed.
But wasn't added support of cursor specific syntax for PostgreSQL with FETCH + limit&offset. It is much more complicated and should be implemented together with cursor's support.
PostgreSQL's format - https://www.postgresql.org/docs/current/sql-select.html#SQL-LIMIT
MySQL's format - https://dev.mysql.com/doc/refman/8.0/en/select.html

Checklist

@Lagovas Lagovas requested a review from Zhaars November 22, 2022 01:15
output: "select * from dual limit 10, 10",
}, { // PostgreSQL format
input: "select * from dual limit all",
output: "select * from dual limit all",
Copy link
Contributor

@Zhaars Zhaars Nov 22, 2022

Choose a reason for hiding this comment

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

As I understand, currently, all of these syntax constructions are supported by both MySQLDialect and PostgreSQLDialect. Does it make sense to throw some error in case of a non-specific DB format?

In this test, we consider select * from dual limit all with MySQLDialect as valid SQL, but it's not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agree, added errors and tests for these cases. additionally I found how we can configure verbosity of loggers in parser and tokenizer. I just decreased verbosity of tokenizer for acra in non-debug mode to avoid logging sensitive parts of SQL query which we cannot control. But now I keep in mind this options that may help in debugging and extending parser.

Copy link
Contributor

@Zhaars Zhaars left a comment

Choose a reason for hiding this comment

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

Lgtm

@Lagovas Lagovas merged commit 0e278a4 into master Nov 22, 2022
@Lagovas Lagovas deleted the lagovas/extend-limit-offset-statements branch November 22, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants