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.

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

sql: address a couple of old TODOs #98823

Merged
merged 1 commit into from
Mar 21, 2023

Commits on Mar 17, 2023

  1. sql: address a couple of old TODOs

    This commit addresses a couple of old TODOs in the
    `connExecutor.dispatchToExecutionEngine` method.
    
    First, it simply removes the now-stale TODO about needing to `Step()`
    the txn before executing the query. The TODO mentioned things about the
    old way FK checks were performed, but we now always run checks in
    a deferred fashion, and perform the `Step`s correctly there for cascades
    and checks, thus, the TODO can be removed. (The TODO itself explicitly
    mentions that it can be removed once we do checks and cascades in the
    deferred fashion.)
    
    Second, it addresses a TODO about how some plan information is saved. Up
    until 961e66f the cleanup of `planNode`
    tree and `flow` infra was intertwined, so in 6ae4881
    in order to "sample" the plan with correct info (some of which is only
    available _after_ the execution is done) we delegated that sampling to
    `planTop.close`. However, with `planNode` tree and `flow` cleanups
    refactored and de-coupled, we no longer have to close the plan early on.
    As a result, we now close the plan in a defer right after we create it
    (now it's the only place we do so), and this commit makes it so that we
    record the plan info explicitly right after returning from the execution
    engine, thus, addressing the second TODO.
    
    Epic: None
    
    Release note: None
    yuzefovich committed Mar 17, 2023
    Configuration menu
    Copy the full SHA
    4d835ec View commit details
    Browse the repository at this point in the history