Skip to content

feat: support cast filter#8034

Draft
discord9 wants to merge 3 commits intomainfrom
feat/scan_simplify_filter
Draft

feat: support cast filter#8034
discord9 wants to merge 3 commits intomainfrom
feat/scan_simplify_filter

Conversation

@discord9
Copy link
Copy Markdown
Contributor

@discord9 discord9 commented Apr 24, 2026

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

#7913 could be one way to fix it, could use together with #7926

What's changed and what's your intention?

support cast(<time index>) <op> <literal> filter in table scan, deal with different precision&off by one error
TODO: more tests

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

Signed-off-by: discord9 <discord9@163.com>
@discord9 discord9 requested a review from a team as a code owner April 24, 2026 13:19
@discord9 discord9 marked this pull request as draft April 24, 2026 13:19
@github-actions github-actions Bot added the docs-not-required This change does not impact docs. label Apr 24, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the ability to extract timestamp ranges from predicates where the time index column is cast to milliseconds. It adds the get_casted_timestamp_filter function and supporting utilities to handle various comparison operators and literal-left expressions, accompanied by unit tests. A review comment suggests simplifying verbose type paths for DataType and TimeUnit by using the existing arrow alias to improve code readability.

Comment on lines +375 to +383
if !matches!(
&cast.data_type,
datatypes::arrow::datatypes::DataType::Timestamp(
datatypes::arrow::datatypes::TimeUnit::Millisecond,
None
)
) {
return false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The type paths for DataType and TimeUnit are unnecessarily verbose. Since datatypes::arrow is imported as arrow at the top of the file, you can use arrow::datatypes::DataType and arrow::datatypes::TimeUnit directly to improve readability.

    if !matches!(
        &cast.data_type,
        arrow::datatypes::DataType::Timestamp(arrow::datatypes::TimeUnit::Millisecond, None)
    ) {
        return false;
    }

Signed-off-by: discord9 <discord9@163.com>
@github-actions github-actions Bot added size/M and removed size/S labels Apr 27, 2026
Signed-off-by: discord9 <discord9@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required This change does not impact docs. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant