Skip to content
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.

Sign up for GitHub

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

Jump to bottom

sql: almost remove context.Context from eval.Context #85782

Merged
merged 4 commits into from
Oct 7, 2022
Merged

sql: almost remove context.Context from eval.Context #85782

merged 4 commits into from
Oct 7, 2022

Commits on Oct 6, 2022

  1. eval: pass context.Context explicitly to Eval 

    This commit plumbs the `context.Context` argument to `Eval` methods
explicitly in order to remove the context field from `eval.Context` in
the follow-up commits. No additional plumbing is performed, so in many
cases `eval.Context.Context` is passed as this new argument.
Additionally it renames `ctx *eval.Context` to `evalCtx *eval.Context`.
This commit is purely a mechanical change.

Release note: None
    @yuzefovich
    yuzefovich committed Oct 6, 2022
    Configuration menu
    Copy the full SHA
    108568e View commit details
    Browse the repository at this point in the history

  2. sql: remove most of eval.Context.Context accesses 

    This commit removes many usages of `eval.Context.Context` in favor of
the newly-plumbed parameter or already available one.

The only change worth calling out is the plumbing of the context for
the optgen-generated `String` implementations - there we use
`context.Background()` since the method is used only for testing and
debugging purposes. This was needed to go around the difficult (and
unnecessary) plumbing into `memo.ScalarFmtInterceptor`.

Release note: None
    @yuzefovich
    yuzefovich committed Oct 6, 2022
    Configuration menu
    Copy the full SHA
    fc1fa24 View commit details
    Browse the repository at this point in the history

  3. sql: remove eval.Context.Ctx() 

    Release note: None
    @yuzefovich
    yuzefovich committed Oct 6, 2022
    Configuration menu
    Copy the full SHA
    3f87f2f View commit details
    Browse the repository at this point in the history

  4. eval: mark stored context.Context as deprecated 

    This commit unexports the stored `context.Context` from `eval.Context`,
marks it as "deprecated" and provides a setter method for now. There
are just two places that still use it - namely
`eval.Context.MustGetPlaceholderValue` and `eval.UnwrapDatum` (when
unwrapping a placeholder), and refactoring those is extremely difficult
since the amount of plumbing required is probably thousands of line
changes without an easy way to automate that. In order to not lose
progress on the removal achieved in the previous commits we do this
deprecation.

The next step for the complete removal is to do analysis of the web of
callsites of these two methods to see whether we have incorrect
layering, and this will be done separately.

Release note: None
    @yuzefovich
    yuzefovich committed Oct 6, 2022
    Configuration menu
    Copy the full SHA
    823ea26 View commit details
    Browse the repository at this point in the history