Skip to content
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

Fix Duration vs Interval comparisons and Interval as LHS #11876

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

samuelcolvin
Copy link
Contributor

@samuelcolvin samuelcolvin commented Aug 7, 2024

Which issue does this PR close?

Close #11891

Rationale for this change

We regularly want to allow queries of the form

SELECT * FROM records WHERE end_timestamp - start_timestamp > interval '2 seconds'

Until now that failed with

type_coercion: Cannot infer common argument type for comparison operation Duration(Microsecond) > Interval(MonthDayNano)

While working on that, I discovered you couldn't compare to an interval with the interval as the left hand side of the operation, so I fixed that too.

What changes are included in this PR?

  • fix Duration vs. Interval comparison - sense check - is it really as simple as it looks?
  • support more operators in machinery to support intervals as the LHF of binary operations
  • add tests for both

Are these changes tested?

Yes

Are there any user-facing changes?

Should be only the above changes to support SQL.

@github-actions github-actions bot added sql SQL Planner logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) labels Aug 7, 2024
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 very much @samuelcolvin

I also filed #11891 to track this

@@ -3109,6 +3109,43 @@ SELECT * FROM VALUES
2024-02-01T08:00:00Z
2023-12-31T23:00:00Z

# interval vs. duration comparison
Copy link
Contributor

Choose a reason for hiding this comment

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

It might also be worth adding tests (could be in a follow on PR) for the other operators enabled in this PR
<=, =, !=, >, and >=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added all cases here.

@alamb alamb merged commit b9bf6c9 into apache:main Aug 8, 2024
24 checks passed
@alamb
Copy link
Contributor

alamb commented Aug 8, 2024

Thanks @samuelcolvin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logical-expr Logical plan and expressions sql SQL Planner sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not compare durations and intervals
2 participants