sql: move AsOfSystemTime to EvalContext#68219
Conversation
|
This change is |
|
I think this was a good idea! I canceled bors-ing my other PR so feel free to re-open this. |
|
feel free to bors this if you accept @rytaft :D im off to bed! |
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @otan)
pkg/sql/conn_executor.go, line 2408 at r2 (raw file):
p.semaCtx.SearchPath = ex.sessionData.SearchPath p.semaCtx.IntervalStyleEnabled = ex.sessionData.IntervalStyleEnabled p.semaCtx.AsOfSystemTime = nil
if you're not going to reset it here, seems like you should actually remove it from the SemaContext, no? I think there are still some references to this field. For example:
cockroach/pkg/sql/create_stats.go
Line 516 in 2eb1635
Previously, AsOfSystemTime was on the SemaCtx. With the bounded staleness work needing this in the EvalContext, and all other contexts accessing this variable have access to EvalContext, move AsOfSystemTime from SemaContext to EvalContext. Release note: None
This reverts commit 25b4d57. No longer needed as AsOfSystemTime is moved to EvalContext. Release note: None
otan
left a comment
There was a problem hiding this comment.
revert also included
Reviewable status:
pkg/sql/conn_executor.go, line 2408 at r2 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
if you're not going to reset it here, seems like you should actually remove it from the
SemaContext, no? I think there are still some references to this field. For example:cockroach/pkg/sql/create_stats.go
Line 516 in 2eb1635
urgh, knew it was too good to be true to pass first go
removed!
rytaft
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r3, 10 of 10 files at r4.
Reviewable status:
|
bors r=rytaft cheers |
|
Build succeeded: |
Previously, AsOfSystemTime was on the SemaCtx. With the bounded
staleness work needing this in the EvalContext, and all other contexts
accessing this variable have access to EvalContext, move
AsOfSystemTime from SemaContext to EvalContext.
Release note: None