search

cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.21k stars 3.82k forks source link

optbuilder: partial index resolution is fragile #61298

Open RaduBerinde opened 3 years ago

RaduBerinde commented 3 years ago

https://github.com/cockroachdb/cockroach/issues/61284 is an example of the fragility around using column resolution by name for partial index predicates. The way this works in the code is hard to understand, as parts of the code that run at different stages interact in subtle ways.

A better alternative would be to build the predicates once, against the "canonical" table column IDs; later, remap the column IDs using CopyAndReplace according to a table ordinal to column ID mapping. I believe that in most cases, the necessary mappings are already available as fetchColIDs, updateColIDs etc.

Jira issue: CRDB-3043

mgartner commented 1 year ago

102909 shows how the same issue affects computed column expressions.

mgartner commented 1 year ago

In the long-term, I think we should build all "synthesized" expressions like this rather than using scopes: computed column expressions, check constraint expressions, partial index expressions, etc. Scopes are good for building explicit expressions in the query which may be malformed, e.g., the have an ambiguous column reference or they reference a non-existent column.

Synthesized expressions are expected to always succeed - the DDL should have failed if not. Scopes aren't the best tool for this. I think the existence of mutationBuilder.disambiguateColumns is proof of this - it's a hack to build synthesized expressions correctly with scopes. If we instead build a canonical expression for the target table, and replace the column references, I think there will be no need to disambiguate columns in a scope.