opt: drop provably-non-null COALESCE operands by lohitkolluri · Pull Request #170858 · cockroachdb/cockroach

lohitkolluri · 2026-05-24T07:28:00Z

This PR was closed in favor of PR #171887. Please refer to that PR for the latest implementation and discussion.

trunk-io · 2026-05-24T07:28:04Z

Merging to master in this repository is managed by Trunk.

To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

blathers-crl · 2026-05-24T07:28:06Z

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

cockroach-teamcity · 2026-05-24T07:28:34Z

This change is

blathers-crl · 2026-05-25T03:23:36Z

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

blathers-crl · 2026-05-25T04:57:34Z

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

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

blathers-crl · 2026-05-25T04:59:14Z

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

mw5h

@mw5h reviewed all commit messages and made 7 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on lohitkolluri).

pkg/sql/opt/norm/scalar_funcs.go line 78 at r1 (raw file):

func (c *CustomFuncs) SimplifyCoalesce(args memo.ScalarListExpr) opt.ScalarExpr {
	for i := 0; i < len(args)-1; i++ {
		item := args[i]

I don't see why we're getting rid of this. It leads to a lot of code churn for no reason.

pkg/sql/opt/norm/scalar_funcs.go line 82 at r1 (raw file):

// SimplifyCoalesceWithNotNullCols is like SimplifyCoalesce, but also discards
// leading operands that are guaranteed to be non-null according to notNullCols.
func (c *CustomFuncs) SimplifyCoalesceWithNotNullCols(

This is dead code.

pkg/sql/opt/norm/scalar_funcs.go line 91 at r1 (raw file):

	args memo.ScalarListExpr, notNullCols opt.ColSet,
) opt.ScalarExpr {
	for i := 0; i < len(args); i++ {

Why are we now examining all the args vs. all but the last we did previously?

pkg/sql/opt/norm/scalar_funcs.go line 114 at r1 (raw file):

// Coalesce expression.
func (c *CustomFuncs) CanSimplifyCoalesce(args memo.ScalarListExpr, notNullCols opt.ColSet) bool {
	simplified := c.simplifyCoalesce(args, notNullCols)

Kind of an expensive way to do this.

pkg/sql/opt/norm/scalar_funcs.go line 128 at r1 (raw file):

	var replace ReplaceFunc
	replace = func(e opt.Expr) opt.Expr {
		if co, ok := e.(*memo.CoalesceExpr); ok {

I don't think this will handle nested Coalesce expressions, so COALESCE(a, COALESCE(b, c)) where b is non-null but a isn't, the outer match guard reports "nothing to simplify" and the inner Coalesce is missed. Either recurse into Coalesce args during the walk, or document the limitation. Worth a test case.

pkg/sql/opt/norm/select_funcs.go line 452 at r1 (raw file):

}

func (c *CustomFuncs) scalarContainsSimplifiableCoalesce(

This belongs in scalar_funcs.go

pkg/sql/opt/norm/testdata/rules/scalar line 192 at r1 (raw file):

# --------------------------------------------------
# SimplifyCoalesceProject
# --------------------------------------------------

Feels like both this and the select tests could benefit from more test cases. Negative tests (doesn't fire unexpectedly), middle argument non-null, maybe interactions with other expressions (IF, CASE, etc)?

mw5h · 2026-05-26T16:24:01Z

Adding @DrewKimball because optimizer rules are tricky and he knows this code well.

blathers-crl · 2026-05-26T17:01:04Z

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
Please ensure your git commit message contains a release note.
When CI has completed, please ensure no errors have appeared.

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

lohitkolluri · 2026-05-28T08:46:57Z

@DrewKimball @mw5h — when you have a moment, could you please take another look?

lohitkolluri · 2026-06-17T18:56:34Z

@DrewKimball @mw5h Hey just following up on this.

DrewKimball

Nice rules!

DrewKimball · 2026-06-22T21:08:06Z

+	newProjections := make(memo.ProjectionsExpr, len(projections))
+	for i := range projections {
+		p := &projections[i]
+		newProjections[i] = c.f.ConstructProjectionsItem(


Ideally, we should only re-build projections (and filters) that actually need this transformation. You could check if the result of SimplifyCoalesceInScalar is pointer-equal to the original projection, and reuse the original ProjectionsItem if it didn't change.

DrewKimball · 2026-06-22T21:21:59Z

+(Project
+    $input:*
+    $projections:* &
+        (CanSimplifyCoalesceInProjections $projections $input)


nit: You can check for at least one simplifiable item in optgen like this:

$projections:[... (ProjectionsItem $scalar) ...] & (CanSimplifyCoalesceInScalar $scalar (NotNullCols $input))

A similar pattern is possible for the Select rule as well.

DrewKimball · 2026-06-22T21:22:36Z

+func (c *CustomFuncs) CanSimplifyCoalesceInProjections(
+	projections memo.ProjectionsExpr, input memo.RelExpr,
+) bool {
+	notNullCols := c.NotNullCols(input)


nit: It would be a bit cleaner to derive the not-null cols once in the optgen rule(s), and then pass them into these helpers.

lohitkolluri · 2026-06-22T21:27:00Z

Thanks for the review.

I'll put up a follow-up commit shortly addressing the feedback.

Apologies if my initial approach introduced some unnecessary churn here. This is one of my first contributions to a codebase of this size, so I'm still learning the optimizer patterns and project conventions. I appreciate the detailed review and guidance.

When a leading COALESCE argument is known non-null from the input's NotNullCols, the remaining arguments are unreachable. Trims leading COALESCE operands in Project projections and Select filters so the simplified plan no longer carries the dropped arguments. Example: SELECT COALESCE(i, j) FROM ij with i NOT NULL no longer keeps the COALESCE in the plan. Release note: None

blathers-crl · 2026-06-22T22:18:37Z

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

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

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

lohitkolluri · 2026-06-22T23:06:24Z

@DrewKimball @mw5h This PR was replaced by #171887 (it had to be closed due to a shallow clone mistake that caused file bloat). Sorry for the trouble please refer to the updated PR i made sure to impliment the suggested changes.

lohitkolluri requested a review from a team as a code owner May 24, 2026 07:28

lohitkolluri requested review from mw5h and removed request for a team May 24, 2026 07:28

blathers-crl Bot added the O-community Originated from the community label May 24, 2026

lohitkolluri mentioned this pull request May 24, 2026

opt: eliminate all arguments of coalesce after the first argument that is guaranteed to be non-null #103596

Open

lohitkolluri force-pushed the opt-coalesce-not-null branch from 96f43bf to 164d43d Compare May 25, 2026 04:59

mw5h requested changes May 26, 2026

View reviewed changes

mw5h requested a review from DrewKimball May 26, 2026 16:23

lohitkolluri requested a review from mw5h May 26, 2026 17:05

lohitkolluri force-pushed the opt-coalesce-not-null branch 2 times, most recently from 51200f1 to eff22df Compare May 26, 2026 17:46

DrewKimball reviewed Jun 22, 2026

View reviewed changes

lohitkolluri closed this Jun 22, 2026

lohitkolluri force-pushed the opt-coalesce-not-null branch from eff22df to abe252e Compare June 22, 2026 22:17

lohitkolluri requested review from a team as code owners June 22, 2026 22:17

lohitkolluri requested review from aa-joshi, angles-n-daemons, jeffswenson, kev-cao and shghasemi and removed request for a team June 22, 2026 22:17

lohitkolluri mentioned this pull request Jun 22, 2026

opt: simplify COALESCE using NOT NULL column derivation #171887

Open

Uh oh!

Conversation

lohitkolluri commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trunk-io Bot commented May 24, 2026

Uh oh!

blathers-crl Bot commented May 24, 2026

Uh oh!

cockroach-teamcity commented May 24, 2026

Uh oh!

blathers-crl Bot commented May 25, 2026

Uh oh!

blathers-crl Bot commented May 25, 2026

Uh oh!

blathers-crl Bot commented May 25, 2026

Uh oh!

mw5h left a comment

Choose a reason for hiding this comment

Uh oh!

mw5h commented May 26, 2026

Uh oh!

blathers-crl Bot commented May 26, 2026

Uh oh!

lohitkolluri commented May 28, 2026

Uh oh!

lohitkolluri commented Jun 17, 2026

Uh oh!

DrewKimball left a comment

Choose a reason for hiding this comment

Uh oh!

DrewKimball Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

DrewKimball Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

DrewKimball Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

lohitkolluri commented Jun 22, 2026

Uh oh!

blathers-crl Bot commented Jun 22, 2026

Uh oh!

lohitkolluri commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lohitkolluri commented May 24, 2026 •

edited

Loading

lohitkolluri commented Jun 22, 2026 •

edited

Loading