sql: check for multiple mutations to the same table by triggers

[DrewKimball]

sql: refactor some cascade/trigger logic

This commit refactors some of the logic shared between cascades and
AFTER triggers. This will make the following commit easier to understand.

Epic: None

Release note: None

sql: check for multiple mutations to the same table by triggers

There are currently some situations where a query that modifies the
same table in multiple locations may cause index corruption (#70731).
To avoid this, we disallow query structures that may lead to a problematic
combination of mutations.

Triggers require special handling to make this check work, because they
can execute arbitrary SQL statements, which can mutate a table directly
or through routines, FK cascades, or other triggers. BEFORE triggers on
the main query "just work" because they are built as UDF invocations as
part of the main query. AFTER triggers and BEFORE triggers fired on
cascades are more difficult, because they are planned lazily only if
the post-query has rows to process.

This commit adds logic to track invalid mutations for both types of
triggers. We now propagate the "ancestor" mutated tables whenever planning
a post-query, so that any triggers planned as part of the post-query
can detect conflicting mutations. See the "After Triggers" section in
statement_tree.go for additional explanation.

Informs #70731

Release note (bug fix): Previously, it was possible to cause index
corruption using AFTER-triggers that fire within a routine. In order
for the bug to manifest, both the AFTER-trigger and the statement that
invokes the routine must mutate the same row of a table with a mutation
other than INSERT.

[blathers-crl]

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[mgartner]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)

---

-- commits line 38 at r2:
Does "routine" mean the trigger routine here? Or the routine that performs the cascade? IIUC this can manifest without calling a UDF or SP directly, which is what a user would probably assume by the expression "invokes the routine".

---

pkg/sql/opt/optbuilder/util.go line 567 at r2 (raw file):

// stmtTreeInitFnForPostQuery returns a function that can be used to initialize
// the statement tree for a post-query plan.
func (b *Builder) stmtTreeInitFnForPostQuery() func() statementTree {

I think this might be better as a method of statementTree. Should it be unit tested?

---

pkg/sql/opt/optbuilder/util.go line 573 at r2 (raw file):

	// Save references to the ancestor statementTreeNodes. Modifications to them
	// after this point should be reflected in the references, ensuring that all
	// ancestor mutations are visible when the post-query is built.

I'm having trouble understanding cases where using mutable references here is necessary. Do you have an exmaple?

[DrewKimball]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mgartner)

---

-- commits line 38 at r2:

Previously, mgartner (Marcus Gartner) wrote…

Does "routine" mean the trigger routine here? Or the routine that performs the cascade? IIUC this can manifest without calling a UDF or SP directly, which is what a user would probably assume by the expression "invokes the routine".

"routine" here means there must be a UDF/SP, with a body statement that mutates the table in an AFTER trigger (or BEFORE trigger on a cascade). It's a pretty convoluted set of steps. I've modified the release note to try and make it more clear, PTAL.

---

pkg/sql/opt/optbuilder/util.go line 567 at r2 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I think this might be better as a method of statementTree. Should it be unit tested?

Done and done.

---

pkg/sql/opt/optbuilder/util.go line 573 at r2 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I'm having trouble understanding cases where using mutable references here is necessary. Do you have an exmaple?

Good callout. It's possible to encounter some mutations in the "current" scope after some mutations in the "child" scope - e.g. if the first branch of a CTE contains a UDF call with a mutation, and the second branch is an INSERT statement. I'll add to the comment. I'll also add a test and verify that it catches a failure to observe all references.