-
Notifications
You must be signed in to change notification settings - Fork 308
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: call df_func with literal #4265
Conversation
WalkthroughThe latest modifications enhance the logging capabilities within the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DfScalarFunction
participant Logger
Client->>DfScalarFunction: Evaluate function
DfScalarFunction->>Logger: info!("Evaluating scalar function: {:?}", self)
DfScalarFunction->>DfScalarFunction: Process function
DfScalarFunction->>Logger: info!("Values: {:?}, Exprs: {:?}", values, exprs)
DfScalarFunction->>Logger: info!("Evaluated values: {:?}", values)
DfScalarFunction->>Logger: info!("Processed with record batch: {:?}", rb)
DfScalarFunction-->>Client: Return result
Poem
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? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/flow/src/expr/scalar.rs (4 hunks)
- src/flow/src/transform/expr.rs (3 hunks)
- tests/cases/standalone/flow/basic.result (1 hunks)
- tests/cases/standalone/flow/basic.sql (1 hunks)
Additional comments not posted (11)
tests/cases/standalone/flow/basic.sql (1)
40-43
: Update flow creation statement.The
CREATE FLOW
statement should be updated to match the new logic for grouping by truncated timestamps.tests/cases/standalone/flow/basic.result (1)
71-75
: Update flow creation result.The result of the
CREATE FLOW
statement should be updated to match the new logic for grouping by truncated timestamps.src/flow/src/expr/scalar.rs (5)
23-23
: Added logging import.The
common_telemetry::info
import has been added to enable logging.
213-214
: Added logging for scalar function evaluation.Logging statements have been added to provide visibility into the evaluation process of DataFusion scalar functions.
217-217
: Added logging for evaluated values.A logging statement has been added to log the evaluated values.
248-251
: Added logging for record batch evaluation.Logging statements have been added to log the record batch used in the evaluation of DataFusion scalar functions.
306-306
: Added logging for decoded scalar function.A logging statement has been added to log the decoded scalar function.
src/flow/src/transform/expr.rs (4)
19-19
: Added logging import.The
common_telemetry::info
import has been added to enable logging.
131-139
: New functionis_proto_literal
.A new function
is_proto_literal
has been added to check if a function argument is a literal.
147-149
: Added literal check inrewrite_scalar_function
.The
rewrite_scalar_function
function has been updated to include a check for literals using theis_proto_literal
function.
162-164
: Added logging for function rewrite.Logging statements have been added to log the function before and after the rewrite.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4265 +/- ##
==========================================
- Coverage 84.96% 84.66% -0.31%
==========================================
Files 1054 1057 +3
Lines 187160 187523 +363
==========================================
- Hits 159026 158769 -257
- Misses 28134 28754 +620 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/flow/src/expr/scalar.rs (4 hunks)
- src/flow/src/transform/expr.rs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/flow/src/expr/scalar.rs
- src/flow/src/transform/expr.rs
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
fix a bug where calling datafusion function from flow with literal args will cause datafusion to throw error with unmatched type
Please explain IN DETAIL what the changes are in this PR and why they are needed:
Checklist
Summary by CodeRabbit
New Features
Improvements