Skip to content

Conversation

@jmaness
Copy link
Contributor

@jmaness jmaness commented Aug 14, 2023

PR for #914. This modifies the parsing of SUBSTRING for dialects like MsSqlServer which doesn't support the SUBSTRING(expr [FROM start] [FOR len]) syntax.

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

This is a really nicely done PR @jmaness -- thank you very much for the contribution. The comments, are clear, as is the code, and the tests thorough. 🏆

#[test]
fn parse_substring() {
one_statement_parses_to("SELECT SUBSTRING('1')", "SELECT SUBSTRING('1')");
let from_for_supported_dialects = TestedDialects {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this is very nice

expr: Box::new(expr),
substring_from: from_expr.map(Box::new),
substring_for: to_expr.map(Box::new),
special: !self.dialect.supports_substring_from_for_expr(),
Copy link
Contributor

Choose a reason for hiding this comment

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

As a minor nit hard coding this to false I think would be clearer for me:

Suggested change
special: !self.dialect.supports_substring_from_for_expr(),
special: false,

Likewise below.

No changes are needed in this PR -- I'll make a follow on one with the proposed change

@alamb alamb merged commit 8bbb853 into apache:main Aug 17, 2023
@jmaness jmaness deleted the feature/iss-914 branch October 6, 2023 19:38
serprex pushed a commit to serprex/sqlparser-rs that referenced this pull request Nov 6, 2023
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.

2 participants