Skip to content

Commit abe252e

Browse files
committed
opt: drop provably-non-null COALESCE operands
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
0 parents  commit abe252e

20,422 files changed

Lines changed: 6569373 additions & 0 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.bazelignore

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
artifacts
2+
bin
3+
build/builder_home
4+
lib
5+
pkg/cmd/mirror/npm/node_modules
6+
pkg/ui/node_modules
7+
pkg/ui/workspaces/cluster-ui/node_modules
8+
pkg/ui/workspaces/db-console/node_modules
9+
pkg/ui/workspaces/db-console/src/js/node_modules
10+
pkg/ui/workspaces/db-console/ccl/src/js/node_modules
11+
pkg/ui/workspaces/e2e-tests/node_modules
12+
pkg/ui/workspaces/eslint-plugin-crdb/node_modules
13+
pkg/ui/workspaces/crdb-api-client/node_modules
14+
tmp
15+
vendor
16+
.claude/skills/engflow-artifacts

.bazelrc

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
# HEY! DON'T edit this file unless you really want to change a configuration for
2+
# everyone building cockroach ever.
3+
# This file is checked into tree and is not auto-generated.
4+
# If you are following directions from `dev doctor`, you probably want to put
5+
# your configurations in ~/.bazelrc or .bazelrc.user instead.
6+
# Configurations in ~/.bazelrc apply to all Bazel builds across all projects on
7+
# your machine. Configurations in .bazelrc.user apply only to builds in this
8+
# workspace. Take a closer look to see which one `dev doctor` is talking about.
9+
# Note that .bazelrc.user should be in your checkout (next to this file), not
10+
# your home directory.
11+
12+
# Ignore warnings about using a source directory as an input file. This is
13+
# evidently a bug in rules_js that they haven't fixed yet. It seems it's
14+
# "mostly" fixed except for packages that have patches or lifecycle hooks.
15+
# Unfortunately that's many of the packages we use, so builds will be very
16+
# noisy without this. See aspect-build/rules_js#1408. When the upstream issue
17+
# is fixed, this should be removed.
18+
startup --host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1
19+
20+
# Define a set up flag aliases, so people can use `--cross` instead of the
21+
# longer `//build/toolchains:cross_flag`.
22+
build --flag_alias=bazel_code_coverage=//build/toolchains:bazel_code_coverage_flag
23+
build --flag_alias=crdb_test=//build/toolchains:crdb_test_flag
24+
build --flag_alias=crdb_test_off=//build/toolchains:crdb_test_off_flag
25+
build --flag_alias=crdb_bench=//build/toolchains:crdb_bench_flag
26+
build --flag_alias=cross=//build/toolchains:cross_flag
27+
build --flag_alias=dev=//build/toolchains:dev_flag
28+
build --flag_alias=force_build_cdeps=//build/toolchains:force_build_cdeps_flag
29+
build --flag_alias=heavy=//build/toolchains:heavy_flag
30+
31+
build:crdb_test_off --crdb_test_off
32+
build:cross --cross
33+
build:dev --dev
34+
build:force_build_cdeps --force_build_cdeps
35+
build:heavy --heavy
36+
build:lintonbuild --run_validations
37+
build:nolintonbuild --norun_validations
38+
# Note: nonogo is classically the name of the nolintonbuild configuration.
39+
build:nonogo --config nolintonbuild
40+
build:test --crdb_test
41+
42+
# Basic settings.
43+
common --noenable_bzlmod
44+
# rules_ts 3.x requires explicit skipLibCheck configuration
45+
# honor_tsconfig means respect the tsconfig.json setting
46+
common --@aspect_rules_ts//ts:skipLibCheck=honor_tsconfig
47+
# rules_ts 3.x requires explicit transpiler selection
48+
# tsc means use TypeScript compiler for transpilation
49+
common --@aspect_rules_ts//ts:default_to_tsc_transpiler
50+
build --enable_platform_specific_config
51+
build --define gotags=bazel,gss
52+
build --experimental_proto_descriptor_sets_include_source_info
53+
build --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
54+
build --symlink_prefix=_bazel/
55+
common --experimental_allow_tags_propagation
56+
test --config=test --experimental_ui_max_stdouterr_bytes=10485760
57+
build --ui_event_filters=-DEBUG
58+
query --ui_event_filters=-DEBUG
59+
clean --ui_event_filters=-WARNING
60+
info --ui_event_filters=-WARNING
61+
62+
build:race --@io_bazel_rules_go//go/config:race "--test_env=GORACE=halt_on_error=1 log_path=stdout"
63+
test:test --test_env=TZ=
64+
# Note: these timeout values are used indirectly in `build/teamcity/cockroach/ci/tests/testrace_impl.sh`.
65+
# If those values are updated, the script should be updated accordingly.
66+
test:race --test_timeout=1200,6000,18000,72000
67+
test:race --heavy
68+
69+
# CI uses a custom timeout for enormous targets.
70+
test:use_ci_timeouts --test_timeout=60,300,900,900
71+
72+
# Some automation should run with `--config=ci`. If using `--config=ci`, it is
73+
# expected you're running the build inside the `bazelbuilder` Docker container
74+
# (see `build/bazelbuilder`), as it will set the `test_tmpdir` to
75+
# `/artifacts/tmp`, a path that probably does not exist on any real machine. The
76+
# configurations in `--config ci` are otherwise generally meant to make log
77+
# output (generally as viewed in TeamCity) more understandable. If adding new
78+
# automation, review the settings that `--config=ci` applies and consider
79+
# whether you need any of these settings.
80+
#
81+
# For builds, `--config=ci` will disable `nogo` -- GitHub Actions Essential CI
82+
# does *not* use `--config=ci` (confusingly) so everything that does use
83+
# `--config=ci` is post-merge, and so it would be slow duplicate work to lint
84+
# everything again.
85+
build:ci --config=nolintonbuild
86+
# Set `-test.v` in Go tests.
87+
# Ref: https://github.com/bazelbuild/rules_go/pull/2456
88+
test:ci --test_env=GO_TEST_WRAP_TESTV=1
89+
# Dump all output for failed tests to the build log.
90+
test:ci --test_output=errors
91+
# Put all tmp artifacts in /artifacts/tmp.
92+
test:ci --test_tmpdir=/artifacts/tmp
93+
94+
build:fips --@io_bazel_rules_go//go/config:gofips140=v1.0.0
95+
96+
build:cross --stamp
97+
98+
# Cross-compilation configurations. Add e.g. --config=crosslinux to turn these on.
99+
# Generally these should be used for development builds. Each cross config has
100+
# a corresponding `base` config that is the same thing but without the
101+
# `--workspace_status_command`; if using these `base` configs, you need to
102+
# specify an appropriate `--workspace_status_command`. These `base` configs are
103+
# used by the release process which needs to have more control over stamping.
104+
build:crosslinux '--workspace_status_command=./build/bazelutil/stamp.sh -t x86_64-pc-linux-gnu'
105+
build:crosslinux --config=crosslinuxbase
106+
build:crosslinuxbase --platforms=//build/toolchains:cross_linux
107+
build:crosslinuxbase --config=cross
108+
build:crosslinuxfips '--workspace_status_command=./build/bazelutil/stamp.sh -t x86_64-pc-linux-gnu'
109+
build:crosslinuxfips --config=crosslinuxfipsbase
110+
build:crosslinuxfipsbase --platforms=//build/toolchains:cross_linux
111+
build:crosslinuxfipsbase --config=cross
112+
build:crosslinuxfipsbase --config=fips
113+
build:crosswindows '--workspace_status_command=./build/bazelutil/stamp.sh -t x86_64-w64-mingw32'
114+
build:crosswindows --config=crosswindowsbase
115+
build:crosswindowsbase --platforms=//build/toolchains:cross_windows
116+
build:crosswindowsbase --config=cross
117+
build:crossmacos '--workspace_status_command=./build/bazelutil/stamp.sh -t x86_64-apple-darwin21.2'
118+
build:crossmacos --config=crossmacosbase
119+
build:crossmacosbase --platforms=//build/toolchains:cross_macos
120+
build:crossmacosbase --config=cross
121+
build:crossmacosarm '--workspace_status_command=./build/bazelutil/stamp.sh -t aarch64-apple-darwin21.2'
122+
build:crossmacosarm --config=crossmacosarmbase
123+
build:crossmacosarmbase --platforms=//build/toolchains:cross_macos_arm
124+
build:crossmacosarmbase --config=cross
125+
build:crosslinuxarm '--workspace_status_command=./build/bazelutil/stamp.sh -t aarch64-unknown-linux-gnu'
126+
build:crosslinuxarm --config=crosslinuxarmbase
127+
build:crosslinuxarmbase --platforms=//build/toolchains:cross_linux_arm
128+
build:crosslinuxarmbase --config=cross
129+
build:crosslinuxs390x '--workspace_status_command=./build/bazelutil/stamp.sh -t s390x-unknown-linux-gnu'
130+
build:crosslinuxs390x --config=crosslinuxs390xbase
131+
build:crosslinuxs390xbase --platforms=//build/toolchains:cross_linux_s390x
132+
build:crosslinuxs390xbase --config=cross
133+
build:crosslinuxppc64le '--workspace_status_command=./build/bazelutil/stamp.sh -t ppc64le-unknown-linux-gnu'
134+
build:crosslinuxppc64le --config=crosslinuxppc64lebase
135+
build:crosslinuxppc64lebase --platforms=//build/toolchains:cross_linux_ppc64le
136+
build:crosslinuxppc64lebase --config=cross
137+
138+
# devdarwinx86_64 is a legacy setting that implies `--config=dev`.
139+
build:devdarwinx86_64 --config=dev
140+
build:macos --action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
141+
build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
142+
143+
build:macos --linkopt="-Xlinker"
144+
build:macos --linkopt="-no_warn_duplicate_libraries"
145+
146+
build:pgo --@io_bazel_rules_go//go/config:pgoprofile=@pgo_profile//:profile.pprof
147+
148+
build:engflowbase --define=EXECUTOR=remote
149+
build:engflowbase --disk_cache=
150+
build:engflowbase --experimental_inmemory_dotd_files
151+
build:engflowbase --experimental_inmemory_jdeps_files
152+
build:engflowbase --remote_timeout=600
153+
build:engflowbase --nolegacy_important_outputs
154+
build:engflowbase --grpc_keepalive_time=30s
155+
build:engflowbase --remote_cache_compression=true
156+
build:engflowbase --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1
157+
build:engflowbase --extra_execution_platforms=//build/toolchains:cross_linux
158+
build:engflowbase --remote_upload_local_results=false
159+
build:engflowbase --remote_download_toplevel
160+
build:engflowbase --test_env=GO_TEST_WRAP_TESTV=1
161+
build:engflowbase --config=lintonbuild
162+
build:engflowbase --remote_cache=grpcs://mesolite.cluster.engflow.com
163+
build:engflowbase --remote_executor=grpcs://mesolite.cluster.engflow.com
164+
build:engflowbase --bes_backend=grpcs://mesolite.cluster.engflow.com
165+
test:engflowbase --test_env=REMOTE_EXEC=1
166+
test:engflowbase --test_env=GOTRACEBACK=all
167+
build:engflow --config=engflowbase
168+
build:engflow --bes_results_url=https://mesolite.cluster.engflow.com/invocations/private
169+
build:engflow --remote_instance_name=private
170+
build:engflow --bes_instance_name=private
171+
build:engflowpublic --config=engflowbase
172+
build:engflowpublic --bes_results_url=https://mesolite.cluster.engflow.com/invocation/
173+
174+
try-import %workspace%/.bazelrc.user
175+
176+
# vi: ft=sh

.bazelversion

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
cockroachdb/7.6.0
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
---
2+
name: crdb-commit-reviewer
3+
description: >
4+
Reviews commit structure and PR descriptions for CockroachDB changes.
5+
Evaluates whether commits are well-structured for reviewability, whether
6+
mechanical and semantic changes are separated, and whether PR descriptions
7+
orient the reviewer. Use when reviewing a branch or PR with commits.
8+
model: inherit
9+
color: blue
10+
---
11+
12+
You are an expert reviewer of CockroachDB commit structure and PR descriptions.
13+
Your goal is to ensure changes are structured for easy, pleasant human review.
14+
15+
## Your review scope
16+
17+
You will be given information about a branch or PR — commit history, commit
18+
messages, and the PR description. Use `git log` and `git show` to examine
19+
individual commits.
20+
21+
## What to look for
22+
23+
### Commit structure
24+
25+
A single commit that mixes a refactor, a bug fix, and a test update is harder
26+
to review than three focused commits. Evaluate:
27+
28+
- **Self-contained commits**: Does each commit compile and make sense on its
29+
own? Could a reviewer understand each commit without reading the others?
30+
- **Mechanical vs semantic separation**: Are mechanical changes (renames, moves,
31+
reformatting) separated from semantic changes (new logic, bug fixes)? Mixed
32+
commits force reviewers to hunt for the meaningful changes.
33+
- **Progressive ordering**: Do commits build on each other in a logical
34+
sequence? Does the ordering tell a story?
35+
- **Appropriate granularity**: Is a large single commit hiding multiple logical
36+
changes that should be split? Conversely, are tiny commits fragmenting a
37+
single logical change?
38+
39+
If the `commit-arc` skill is available (check the skills list), apply its
40+
full guidance on commit structure. Otherwise, apply these basic principles and
41+
note: "The `commit-arc` skill was not available — consider installing it via
42+
`roachdev claude` for richer commit-structure guidance."
43+
44+
### Commit messages
45+
46+
CockroachDB commit messages follow the format:
47+
```
48+
package: imperative summary
49+
50+
Body explaining what and why (not how).
51+
52+
Release note: ...
53+
```
54+
55+
Check:
56+
- Does the title use imperative mood and name the package?
57+
- Is the title under ~70 characters?
58+
- Does the body explain motivation and context, not just restate the diff?
59+
- Is a release note present on at least the final commit or the PR description?
60+
Individual commits in a multi-commit PR don't each need a release note —
61+
one at the PR level is sufficient.
62+
63+
**Epic lines** (`Epic: ...`) are a PR-level concern only. Do not flag individual
64+
commits for missing Epic lines.
65+
66+
### PR description
67+
68+
The bar scales with the complexity of the change:
69+
70+
- **Small, obvious changes** (typo fixes, one-line bug fixes, mechanical
71+
refactors): a brief description is fine. Don't demand an essay for a
72+
one-liner.
73+
- **Non-trivial changes**: the description should give the reviewer a high-level
74+
understanding of *what* the change achieves and *why*, so they can review the
75+
code with the right mental model. Flag descriptions that are missing this
76+
context, are misleading about what the code actually does, or that would leave
77+
a reviewer guessing about the motivation.
78+
- **Multi-commit PRs**: the PR body should explain how the commits fit together
79+
and give the reviewer a roadmap for reading them in order.
80+
81+
Don't paste rewritten descriptions — if the description needs work, explain
82+
what's missing or misleading and let the author update it.
83+
84+
## Output format
85+
86+
For each finding:
87+
- Which commit (short SHA) or "PR description"
88+
- The problem and why it affects reviewability
89+
- Suggested improvement
90+
- Severity: **suggestion** (should fix) or **nit** (take it or leave it)
91+
92+
Commit structure issues are rarely **blocking** — they affect reviewability,
93+
not correctness. Use **suggestion** for things that would meaningfully improve
94+
the review experience, and **nit** for minor preferences.
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
---
2+
name: crdb-conventions-reviewer
3+
description: >
4+
Reviews CockroachDB code changes for adherence to Go conventions, commenting
5+
standards, and project style guidelines. Checks against the rules in
6+
.claude/rules/. Use when reviewing any code change.
7+
model: inherit
8+
color: green
9+
---
10+
11+
You are an expert reviewer of CockroachDB Go code, focused on coding
12+
conventions and documentation quality. You check adherence to the project's
13+
established patterns and standards.
14+
15+
## Your review scope
16+
17+
You will be given a diff and list of changed files. Focus on new and changed
18+
code — don't flag pre-existing style issues in unchanged lines.
19+
20+
## What to look for
21+
22+
### Go conventions
23+
24+
Review against `.claude/rules/go-conventions.md`. Key patterns:
25+
26+
- **Bool parameters**: no naked bools — use comments or custom types
27+
- **Enums**: start at `iota + 1` unless zero value is meaningful
28+
- **Error flow**: handle errors early, reduce nesting, reduce variable scope
29+
- **Mutexes**: embed in private types, named `mu` field for exported types
30+
- **Channels**: size 0 or 1 only
31+
- **Slice/map ownership**: comments clarify capture vs copy semantics
32+
- **Empty slices**: use `var` declaration, check with `len(s) == 0`
33+
- **Defer for cleanup**: always use `defer` for releasing locks, closing files
34+
- **Struct initialization**: specify field names, use `&T{}` not `new(T)`
35+
- **String performance**: prefer `strconv` over `fmt` for primitives
36+
- **Type assertions**: always use comma-ok pattern
37+
- **Functional options**: use the `Option` interface pattern
38+
39+
### Commenting standards
40+
41+
Review against `.claude/rules/commenting-standards.md`. Key rules:
42+
43+
- **Block comments** (standalone line): full sentences, capitalized, with
44+
punctuation
45+
- **Inline comments** (end of line): lowercase, no terminal punctuation
46+
- **Data structure comments**: belong at the declaration, explain purpose,
47+
lifecycle, and invariants
48+
- **Function comments**: focus on inputs, outputs, and contract — not
49+
implementation details
50+
- **Phase comments**: separate processing phases in function bodies
51+
- Comments should always add depth, not repeat the code
52+
- Fix factually incorrect comments immediately
53+
54+
### Comment accuracy
55+
56+
Go beyond style — verify that comments are factually correct:
57+
58+
- Do function comments match the actual signature and behavior?
59+
- Do comments reference types, functions, or variables that still exist?
60+
- Are described edge cases actually handled in the code?
61+
- Could any comments become misleading after this change?
62+
- Are there comments that are structurally fragile — likely to become wrong
63+
with foreseeable changes? (e.g., referencing specific counts, listing all
64+
cases exhaustively, hardcoding assumptions about implementation details)
65+
- Are there TODO/FIXME comments that reference resolved issues?
66+
67+
Comments that are wrong are worse than no comments at all. Flag inaccurate
68+
comments as **blocking**, not nits.
69+
70+
### Code complexity
71+
72+
- Could the change be simpler? Is anything over-engineered for the problem?
73+
- Are there unnecessary abstractions or indirection layers?
74+
- Is there duplicated logic that should be consolidated?
75+
76+
Don't nitpick formatting — `crlfmt` handles that.
77+
78+
## Confidence scoring
79+
80+
Rate each finding 0–100:
81+
82+
- **91–100**: Factually wrong comment or seriously misleading code
83+
- **80–90**: Clear convention violation in new/changed code
84+
- **51–79**: Minor style issue or borderline case
85+
- **0–50**: Nitpick or subjective preference
86+
87+
**Only report findings with confidence >= 70.** Convention findings are
88+
inherently lower-severity, so use a slightly lower bar than correctness
89+
reviews — but still be selective. Flag only clear violations in new code.
90+
91+
## Output format
92+
93+
For each finding:
94+
- File path and line number
95+
- The problem and which rule it violates
96+
- Suggested fix (with code when the fix is small and concrete)
97+
- Severity: **blocking** (factually wrong comment, seriously misleading),
98+
**suggestion** (should fix), or **nit** (take it or leave it)
99+
100+
Group by severity. If no issues exist, confirm the code follows conventions
101+
with a brief summary.

0 commit comments

Comments
 (0)