Skip to content

Start new line if \r in Postgres dialect #1647

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

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

hansott
Copy link
Contributor

@hansott hansott commented Jan 7, 2025

Currently the tokenizer throws an error for

insert into cats_2 (petname) values ('foo'),--\r(version()||'\n');

this is because postgres treats \r as a separate new line character, see https://github.com/postgres/postgres/blob/master/src/backend/parser/scan.l

In order to make the world safe for Windows and Mac clients as well as Unix ones, we accept either \n or \r as a newline.
A DOS-style \r\n sequence will be seen as two successive newlines, but that doesn't cause any problems.
non_newline [^\n\r]
comment ("--"{non_newline}*)

Let's make sure we start a new line if we encounter a \r when tokenizing a comment.

Currently the tokenizer throws an error for

insert into cats_2 (petname) values ('foo'),--\r(version()||'\n');

this is because postgres treats \r as a separate new line character, see https://github.com/postgres/postgres/blob/master/src/backend/parser/scan.l

> In order to make the world safe for Windows and Mac clients as well as Unix ones, we accept either \n or \r as a newline.
> A DOS-style \r\n sequence will be seen as two successive newlines, but that doesn't cause any problems.
> non_newline			[^\n\r]
> comment			("--"{non_newline}*)

Let's make sure we start a new line if we encounter a \r when tokenizing a comment.
…o patch-carriage-return

* 'main' of github.com:hansott/datafusion-sqlparser-rs:
  Add support for MySQL's INSERT INTO ... SET syntax (apache#1641)
  Add support for Snowflake LIST and REMOVE (apache#1639)
  Add support for the SQL OVERLAPS predicate (apache#1638)
  Add support for various Snowflake grantees (apache#1640)
  Add support for USE SECONDARY ROLE (vs. ROLES) (apache#1637)
  Correctly tokenize nested comments (apache#1629)
  Add support for MYSQL's `RENAME TABLE` (apache#1616)
  Test benchmarks and Improve benchmark README.md (apache#1627)
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.

Thank you @hansott - this makes sense to me and is a nice contribution

I'll leave it open for a while in case @iffyio would also like a chance to review

@alamb alamb changed the title Start new line if \r and dialect is postgres Start new line if \r in Postgres dialect Jan 7, 2025
Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @hansott!

@iffyio iffyio merged commit 0cd49fb into apache:main Jan 7, 2025
9 checks passed
@hansott hansott deleted the patch-carriage-return branch January 7, 2025 19:12
ayman-sigma pushed a commit to sigmacomputing/sqlparser-rs that referenced this pull request Apr 10, 2025
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