Skip to content

Fix parsing of :: cast after parenthesized DEFAULT expression #2168

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.

Sign up for GitHub

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

Open
isaacparker0 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix parsing of :: cast after parenthesized DEFAULT expression #2168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2598da
Fix parsing of :: cast after parenthesized DEFAULT expression
isaacparker0 Jan 21, 2026
7aaa839
More comprehensive tests
isaacparker0 Jan 30, 2026
2f20af6
Add suggested test case
isaacparker0 Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9002,7 +9002,15 @@ impl<'a> Parser<'a> {
/// [ColumnOption::NotNull].
fn parse_column_option_expr(&mut self) -> Result<Expr, ParserError> {
if self.peek_token_ref().token == Token::LParen {
let expr: Expr = self.with_state(ParserState::Normal, |p| p.parse_prefix())?;
let mut expr = self.with_state(ParserState::Normal, |p| p.parse_prefix())?;
Copy link
Contributor

@iffyio iffyio Jan 29, 2026

Choose a reason for hiding this comment

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

Suggested change
let mut expr = self.with_state(ParserState::Normal, |p| p.parse_prefix())?;
let mut expr = self.with_state(ParserState::Normal, |p| p.parse_subexpr())?;

not sure I followed the intent of the added code, but I wonder would the issue be fixed by changing this line instead (thinking since (foo())::INT should be parsed as an expression), or are there other considerations?

Copy link
Contributor Author

@isaacparker0 isaacparker0 Jan 29, 2026

Choose a reason for hiding this comment

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

That change would fix the bug here, but break the behavior added in #1927. In Normal state, NOT NULL is treated as an expression (i.e., IS NOT NULL), but in ColumnDefinition state it's not, allowing the trailing NOT NULL to be parsed as a column constraint instead.

My proposed change is basically just duplicating part of the existing logic used in parse_subexpr1. If we run parse_prefix in normal state but the infix loop in ColumnDefinition state, we maintain the NOT NULL parsing behavior added in the previous PR but still properly handle infix operators like ::TEXT.

I'm still familiarizing myself with this repo, so there may be a better way to do this, but this is what I've come up with so far and why it seems like the right approach to me.

Also happy to do a bit of refactoring to cleanly extract a single shared logic for the part of parse_subexpr that we are duplicating if that seems preferable.

Footnotes

  1. https://github.com/apache/datafusion-sqlparser-rs/blob/62cf16f3ece6f3d5985e35893407c8db359ffd3f/src/parser/mod.rs#L1337-L1363 ↩

Copy link
Contributor

@iffyio iffyio Jan 30, 2026

Choose a reason for hiding this comment

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

could we do something like this instead?

let expr = self.with_state(ParserState::Normal, |p| p.parse_subexpr(self.dialect.prec_value(Precedence::DoubleColon)))?;
if self.consume_token(&Token::DoubleColon) {
	Ok(Expr::Cast {
    	kind: CastKind::DoubleColon,
        expr: Box::new(expr),
        data_type: self.parse_data_type()?,
        format: None,
    })
} else {
	Ok(expr)
}

Copy link
Contributor Author

@isaacparker0 isaacparker0 Jan 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This approach would only handle the :: case. There could be other infix operators or chained casts, which my current approach handles properly by leveraging the full existing infix loop.

I added additional test cases to document this behavior.

Copy link
Contributor

@iffyio iffyio Jan 30, 2026

Choose a reason for hiding this comment

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

to support the chained cast, we would consume the double colon in a loop? I think the current approach duplicates parts of the expr parsing code most of which aren't exactly related to the problem being solved for, which makes it not ideal

Copy link
Contributor Author

@isaacparker0 isaacparker0 Jan 30, 2026

Choose a reason for hiding this comment

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

What about a non :: operator, such as CREATE TABLE t (c INT DEFAULT (foo()) + 1)? (I just added a test case for this)

Admittedly, we are getting into the territory of obscure but technically legal postgres syntax, but it still seems like we should solve this in a more general way than hardcoding one specific operator.

Copy link
Contributor

@iffyio iffyio Jan 31, 2026

Choose a reason for hiding this comment

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

What about a non :: operator, such as CREATE TABLE t (c INT DEFAULT (foo()) + 1)?

the precedence operator should handle this case?

Copy link
Contributor

@iffyio iffyio Jan 31, 2026

Choose a reason for hiding this comment

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

Im thinking a generic version would need to use sub_expr somehow, ideally we find a way to reuse the core expr parsing loop

expr = self.parse_compound_expr(expr, vec![])?;
loop {
let next_precedence = self.get_next_precedence()?;
if next_precedence == 0 || self.peek_token_ref().token == Token::Period {
break;
}
expr = self.parse_infix(expr, next_precedence)?;
}
Ok(expr)
} else {
Ok(self.parse_expr()?)
Expand Down
12 changes: 12 additions & 0 deletions tests/sqlparser_postgres.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,18 @@ fn parse_create_table_with_defaults() {
}
}

#[test]
fn parse_infix_after_parenthesized_column_option() {
// Cast
pg().verified_stmt("CREATE TABLE t (c TEXT DEFAULT (foo())::TEXT)");
// Chained casts
pg().verified_stmt("CREATE TABLE t (c TEXT DEFAULT (foo())::INT::TEXT)");
// Other infix operators
pg().verified_stmt("CREATE TABLE t (c INT DEFAULT (foo()) + 1)");
// Cast with NOT NULL constraint
pg().verified_stmt("CREATE TABLE t (c TEXT DEFAULT (foo())::TEXT NOT NULL)");
}

#[test]
fn parse_create_table_from_pg_dump() {
let sql = "CREATE TABLE public.customer (
Expand Down