SessionContext from ExecutionEngine #960
Conversation
The value is provided by ExecutionEngine at the start of execution. This can be improved by changing PhysicalPlan.next to return a tuple of (ResultSet, SessionContext) and adding a SessionContextPlan that just returns (<Empty>, SessionContext<Time>) as a leaf plan. Signed-off-by: MaxKsyunz <[email protected]>
|
@penghuo what do you think of this implementation of query context (I called it |
| @@ -19,8 +20,11 @@ public interface Expression extends Serializable { | |||
| /** | |||
| * Evaluate the value of expression in the value environment. | |||
| */ | |||
| ExprValue valueOf(Environment<Expression, ExprValue> valueEnv); | |||
| ExprValue valueOf(Environment<Expression, ExprValue> valueEnv, SessionContext sessionContext); | |||
There was a problem hiding this comment.
Does it make sense to provide an overloaded version of valueOf that takes just an Environment?
ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
return valueOf(valueEnv, SessionContext.None);
}
This would cut down the number of changes needed in this PR
| * @param valueEnv : Dataset to parse value from. | ||
| * | ||
| * @param valueEnv : Dataset to parse value from. | ||
| * @param sessionContext |
There was a problem hiding this comment.
can you add a comment for what sessionContext is?
| private final Clock epochStartClock = Clock.fixed(Instant.EPOCH, ZoneId.of("UTC")); | ||
| @Override | ||
| public Clock getQueryStartClock() { | ||
| return epochStartClock; |
There was a problem hiding this comment.
I'd prefer if this throws a runtime exception since it shouldn't be used.
There was a problem hiding this comment.
That being said, it might be better to use Optionals and this default class has an empty clock defined.
|
|
||
| @Override | ||
| public Clock getSystemClock() { | ||
| return epochStartClock; |
There was a problem hiding this comment.
I'd prefer if this throws a runtime exception since it shouldn't be used.
|
Will revisit this later. |
Implement now function using
SessionContextmanaged by the execution engine. Unlike #937 this implementation decouples the function definition from capturing query start time.In addition, this declaration of
Expression.valueOfallows for passing other metadata to function implementations.Implementation overview
SessionContextis a class that stores query execution start time.It is provided to now implementation through an argument to
Expression.valueOf.That is done by
ProjectOperatorwhile executingExecutionEngineprovides an instance ofSessionContextto eachPhyiscalPlanby callingPhysicalPlan.open.Remaining work
Ensure that
SessionContextis passed in all instances where an expression can containnow()call.Signed-off-by: MaxKsyunz [email protected]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.