Set query_execution_start_time on snapshot from SessionContext (#4747) #4750
Conversation
github-actions
bot
added
core
| @@ -44,14 +44,16 @@ impl ExecutionProps { | |||
| /// Creates a new execution props | |||
| pub fn new() -> Self { | |||
| ExecutionProps { | |||
| query_execution_start_time: chrono::Utc::now(), | |||
| // Set this to a fixed sentinel to make it obvious if this is | |||
| // not being updated / propagated correctly | |||
| self.create_physical_plan_impl().await | ||
| } | ||
|
|
||
| /// Temporary pending #4626 |
There was a problem hiding this comment.
I no longer intend to do this, as it creates far too much churn to justify it, this PR instead opts to workaround the issue by punting responsibility for setting the execution start time onto the caller if they're using the lower-level APIs.
| pub fn state(&self) -> SessionState { | ||
| self.state.read().clone() | ||
| let mut state = self.state.read().clone(); | ||
| state.execution_props.start_execution(); |
There was a problem hiding this comment.
This is a nice change 👍 make sense
|
Benchmark runs are scheduled for baseline = 5630339 and contender = 981a9bb. 981a9bb is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #4747
Closes #4626
Rationale for this change
This works around #4747 by ensuring that the execution time is already set by the time a user has obtained a
SessionState.What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?