Closed etolbakov closed 3 weeks ago
Attention: Patch coverage is 97.29730%
with 4 lines
in your changes missing coverage. Please review.
Project coverage is 84.76%. Comparing base (
4b42c7b
) to head (a8435dc
).
I see that one sqlness
test is failing
Result unexpected, path:"./tests/cases/standalone/common/types/interval/interval.sql"
[
... skip(287) ...,
~ Error: 3000(PlanQuery), Failed to plan SQL: Error during planning: No function matches the given name and argument types 'SUM(Interval(MonthDayNano))'. You might need to add explicit type casts. -> Error: 3000(PlanQuery), Failed to plan SQL: Error during planning: Execution error: User-defined coercion failed with Execution("Sum not supported for Interval(MonthDayNano)") and No function matches the given name and argument types 'SUM(Interval(MonthDayNano))'. You might need to add explicit type casts.,
Candidate functions:,
~ SUM(Int8/Int16/Int32/Int64/UInt8/UInt16/UInt32/UInt64/Float32/Float64) -> SUM(UserDefined),
... skip(11) ...
]
strangely enough I don't see the same behaviour running those tests locally. Keep looking though
@etolbakov Could you rebase the branch with the main branch? Looks like it's an issue after upgrading DataFusion and fixed.
🤔
I've just synced with the latest upstream in my branch and run sqlness
tests locally, interval
is passing
Test case "/Users/eugenet/Documents/projects/other/rust/greptimedb/tests/cases/standalone/common/types/float/nan_ordering" finished, cost: 221ms
Test case "/Users/eugenet/Documents/projects/other/rust/greptimedb/tests/cases/standalone/common/types/float/nan_window" finished, cost: 148ms
Test case "/Users/eugenet/Documents/projects/other/rust/greptimedb/tests/cases/standalone/common/types/interval/interval" finished, cost: 165ms
Test case "/Users/eugenet/Documents/projects/other/rust/greptimedb/tests/cases/standalone/common/types/string/bigstring" finished, cost: 109ms
standalone log file at /var/folders/7l/7j6kdl01359dqfh_wz6sztgm0000gp/T/sqlnessMgeUNN/greptime-sqlness-standalone.log
Looks like it's an issue after upgrading DataFusion and fixed.
seems that the error message format became different.
Error: 3000(PlanQuery), Failed to plan SQL: Error during planning: Execution error: User-defined coercion failed with Execution("Sum not supported for Interval(MonthDayNano)") and No function matches the given name and argument types 'SUM(Interval(MonthDayNano))'. You might need to add explicit type casts.
Candidate functions:
SUM(UserDefined)
@etolbakov Let me try it on my machine, thank you 👍
Did you forget to commit interval.result
?
Yes, the error message is different after upgrading DataFusion, you have to commit and push it too.
My result is,
Error: 3000(PlanQuery), Failed to plan SQL: Error during planning: Execution error: User-defined coercion failed with Execution("Sum not supported for Interval(MonthDayNano)") and No function matches the given name and argument types 'SUM(Interval(MonthDayNano))'. You might need to add explicit type casts.
Candidate functions:
SUM(UserDefined)
It's different from your branch result.
Thank you very much for validating! let me double check now (maybe it's me being inattentive)
This update introduces a new transformer rule for SQL statements to expand short interval names to their full names, such as converting 'h' to 'hours'. Additionally, it includes new SQL test cases to validate these changes, and a minor comment update regarding type aliases in a different module.
File/Directory | Change Summary |
---|---|
src/sql/src/statements/transform.rs |
Introduced expand_interval module and added ExpandIntervalTransformRule to the RULES vector. |
src/sql/src/statements/transform/expand_interval.rs |
Created new module to expand short interval names to full names. Implemented ExpandIntervalTransformRule . |
src/sql/src/statements/transform/type_alias.rs |
Updated comments related to the Int8 alias for Postgres Bigint in the replace_type_alias function. |
tests/cases/standalone/common/types/interval/interval.result |
Added SQL queries demonstrating various interval operations. |
tests/cases/standalone/common/types/interval/interval.sql |
Added SQL queries selecting various intervals. |
sequenceDiagram
participant SQLParser
participant TransformEngine
participant ExpandIntervalTransformRule
participant SQLExecutor
SQLParser->>TransformEngine: Parse query
TransformEngine->>ExpandIntervalTransformRule: Apply expansion rule
ExpandIntervalTransformRule-->>TransformEngine: Expanded intervals
TransformEngine->>SQLExecutor: Transformed query
SQLExecutor->>SQLExecutor: Execute query
In the land of SQL statements,
New intervals take their place.
No more just 'h' or 'm' or 's',
Full names now fill the space.
Expanding intervals clear and bright,
Bringing queries to full light. 🎉
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
@waynexia @fengjiachun Please take a look.
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link
https://github.com/GreptimeTeam/greptimedb/issues/4168
What's changed and what's your intention?
improve interval expression by handling shortened version
Checklist
Summary by CodeRabbit
New Features
Documentation
Int8
alias for PostgresBigint
.Tests