Skip to content

Conversation

@trungda
Copy link
Contributor

@trungda trungda commented Jan 30, 2024

Which issue does this PR close?

Closes #9060

Rationale for this change

For non-numeric/non-interval/non-timestamp types, we should gracefully handle and return a meaningful error message.

What changes are included in this PR?

Handle error instead of panicking.

Are these changes tested?

added unit test + sqllogictest + manually

❯ select -'100';
Error during planning: Negation only supports numeric, interval and timestamp types

Are there any user-facing changes?

@github-actions github-actions bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Jan 30, 2024
@trungda trungda marked this pull request as ready for review January 30, 2024 18:00
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 @trungda 🙏

}

#[test]
fn test_negation_valid_types() -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

👌 very nice -- thank you

trungda and others added 2 commits January 30, 2024 13:32
Use `plan_err!(..)`

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@alamb alamb merged commit 9fbea3c into apache:main Jan 31, 2024
@alamb
Copy link
Contributor

alamb commented Jan 31, 2024

Thanks again @trungda 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Internal error when negating strings

2 participants