Skip to content

sql: add parser grammar for DROP PROVISIONED ROLES - #166923

Closed
souravcrl wants to merge 2 commits into
cockroachdb:masterfrom
souravcrl:fork/drop-provisioned-roles-parser
Closed

sql: add parser grammar for DROP PROVISIONED ROLES#166923
souravcrl wants to merge 2 commits into
cockroachdb:masterfrom
souravcrl:fork/drop-provisioned-roles-parser

Conversation

@souravcrl

@souravcrl souravcrl commented Mar 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Extend the SQL parser grammar to support the DROP PROVISIONED ROLES
statement with optional WITH clauses and LIMIT:

DROP PROVISIONED ROLES [WITH ] [LIMIT ]

Options (comma-separated):
SOURCE =
LAST LOGIN BEFORE

Add PROVISIONED as an unreserved keyword. Define dedicated grammar
rules (opt_with_drop_provisioned_roles_options,
drop_provisioned_roles_options_list, drop_provisioned_roles_option)
and wire the new drop_provisioned_roles_stmt production into drop_stmt.

Add parse roundtrip tests covering all option combinations, including
error tests for duplicate option detection.

Fix a pre-existing format string bug in combineExpr and
combineStringOrPlaceholderOptList where % was used instead of %s,
producing garbled error messages for duplicate options.

Epic CRDB-52460

fixes: CRDB-52797

Release note: None

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@souravcrl
souravcrl requested a review from a team March 27, 2026 18:03
@trunk-io

trunk-io Bot commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

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.

@cockroach-teamcity

cockroach-teamcity commented Mar 27, 2026

Copy link
Copy Markdown
Member

This change is Reviewable

@cockroach-teamcity

cockroach-teamcity commented Mar 27, 2026

Copy link
Copy Markdown
Member
⚪ Sysbench [SQL, 3node, oltp_read_write]
Metric Old Commit New Commit Delta Note
⚪ sec/op 9.887m ±4% 9.842m ±5% ~ p=0.806 n=15
⚪ allocs/op 8.090k ±0% 8.080k ±0% ~ p=0.213 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/7d4afec/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7d4afece35c40ce2f60116aaa3ba54488ed4e291/bin/pkg_sql_tests benchdiff/7d4afec/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7d4afec/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/d59acc6/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/d59acc6b0a891f6535267bcb2c34b9a483bd38d7/bin/pkg_sql_tests benchdiff/d59acc6/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/d59acc6/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=d59acc6 --new=7d4afec --memprofile ./pkg/sql/tests
🔴 Sysbench [KV, 3node, oltp_read_only]
Metric Old Commit New Commit Delta Note
🔴 sec/op 3.098m ±1% 3.130m ±0% +1.06% p=0.000 n=15
⚪ allocs/op 2.106k ±0% 2.106k ±0% ~ p=0.671 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/7d4afec/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7d4afece35c40ce2f60116aaa3ba54488ed4e291/bin/pkg_sql_tests benchdiff/7d4afec/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7d4afec/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/d59acc6/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/d59acc6b0a891f6535267bcb2c34b9a483bd38d7/bin/pkg_sql_tests benchdiff/d59acc6/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/d59acc6/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_read_only$ --old=d59acc6 --new=7d4afec --memprofile ./pkg/sql/tests
⚪ Sysbench [KV, 3node, oltp_write_only]
Metric Old Commit New Commit Delta Note
⚪ sec/op 2.975m ±1% 2.990m ±0% ~ p=0.056 n=15
⚪ allocs/op 4.211k ±0% 4.211k ±0% ~ p=0.910 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/7d4afec/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7d4afece35c40ce2f60116aaa3ba54488ed4e291/bin/pkg_sql_tests benchdiff/7d4afec/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7d4afec/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/d59acc6/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/d59acc6b0a891f6535267bcb2c34b9a483bd38d7/bin/pkg_sql_tests benchdiff/d59acc6/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/d59acc6/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_write_only$ --old=d59acc6 --new=7d4afec --memprofile ./pkg/sql/tests
Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/7d4afece35c40ce2f60116aaa3ba54488ed4e291/23660531214-1/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/d59acc6b0a891f6535267bcb2c34b9a483bd38d7/23660531214-1/\* old/

built with commit: 7d4afece35c40ce2f60116aaa3ba54488ed4e291

@cockroach-teamcity cockroach-teamcity added the X-perf-check Microbenchmarks CI: Added to a PR if a performance regression is detected and should be checked label Mar 27, 2026
@souravcrl
souravcrl force-pushed the fork/drop-provisioned-roles-parser branch from 7d4afec to 34b247f Compare April 8, 2026 07:17
@souravcrl
souravcrl marked this pull request as ready for review April 8, 2026 07:18
@souravcrl
souravcrl requested a review from a team as a code owner April 8, 2026 07:18
@souravcrl
souravcrl requested review from a team, DrewKimball and sanchit-CRL and removed request for a team April 8, 2026 07:18
@souravcrl
souravcrl force-pushed the fork/drop-provisioned-roles-parser branch from 34b247f to 7e755db Compare April 8, 2026 07:43
@souravcrl
souravcrl requested a review from a team as a code owner April 8, 2026 07:43
@cockroach-teamcity

cockroach-teamcity commented Apr 8, 2026

Copy link
Copy Markdown
Member
⚪ Sysbench [SQL, 3node, oltp_read_write]
Metric Old Commit New Commit Delta Note
⚪ sec/op 10.01m ±2% 9.787m ±4% ~ p=0.744 n=15
⚪ allocs/op 8.098k ±1% 8.081k ±1% ~ p=0.280 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/34b247f/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/34b247f5bf43d32a7fb20b3ff622f41bf88deb88/bin/pkg_sql_tests benchdiff/34b247f/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/34b247f/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=905f7e3 --new=34b247f --memprofile ./pkg/sql/tests
⚪ Sysbench [KV, 3node, oltp_read_only]
Metric Old Commit New Commit Delta Note
⚪ sec/op 3.020m ±0% 3.016m ±1% ~ p=0.713 n=15
⚪ allocs/op 2.106k ±0% 2.106k ±0% ~ p=0.657 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/34b247f/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/34b247f5bf43d32a7fb20b3ff622f41bf88deb88/bin/pkg_sql_tests benchdiff/34b247f/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/34b247f/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_read_only$ --old=905f7e3 --new=34b247f --memprofile ./pkg/sql/tests
⚪ Sysbench [KV, 3node, oltp_write_only]
Metric Old Commit New Commit Delta Note
⚪ sec/op 2.856m ±0% 2.841m ±0% -0.50% p=0.001 n=15
⚪ allocs/op 4.201k ±0% 4.197k ±0% -0.10% p=0.011 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/34b247f/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/34b247f5bf43d32a7fb20b3ff622f41bf88deb88/bin/pkg_sql_tests benchdiff/34b247f/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/34b247f/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_write_only$ --old=905f7e3 --new=34b247f --memprofile ./pkg/sql/tests
Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/34b247f5bf43d32a7fb20b3ff622f41bf88deb88/24123026689-1/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/905f7e3442bee845401cfbb4600fab828e348665/24123026689-1/\* old/

built with commit: 34b247f5bf43d32a7fb20b3ff622f41bf88deb88

@cockroach-teamcity

cockroach-teamcity commented Apr 8, 2026

Copy link
Copy Markdown
Member
⚪ Sysbench [SQL, 3node, oltp_read_write]
Metric Old Commit New Commit Delta Note
⚪ sec/op 9.856m ±3% 9.861m ±4% ~ p=0.367 n=15
⚪ allocs/op 8.117k ±1% 8.084k ±1% ~ p=0.221 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/7e755db/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7e755dbdbcf0093f7ab792396ca99edebd740ee2/bin/pkg_sql_tests benchdiff/7e755db/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7e755db/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=905f7e3 --new=7e755db --memprofile ./pkg/sql/tests
⚪ Sysbench [KV, 3node, oltp_read_only]
Metric Old Commit New Commit Delta Note
⚪ sec/op 3.063m ±2% 3.062m ±1% ~ p=0.567 n=15
⚪ allocs/op 2.106k ±0% 2.106k ±0% ~ p=0.590 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/7e755db/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7e755dbdbcf0093f7ab792396ca99edebd740ee2/bin/pkg_sql_tests benchdiff/7e755db/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7e755db/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_read_only$ --old=905f7e3 --new=7e755db --memprofile ./pkg/sql/tests
🟢 Sysbench [KV, 3node, oltp_write_only]
Metric Old Commit New Commit Delta Note
🟢 sec/op 2.883m ±1% 2.856m ±0% -0.95% p=0.000 n=15
⚪ allocs/op 4.202k ±0% 4.199k ±0% ~ p=0.093 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/7e755db/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7e755dbdbcf0093f7ab792396ca99edebd740ee2/bin/pkg_sql_tests benchdiff/7e755db/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7e755db/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_write_only$ --old=905f7e3 --new=7e755db --memprofile ./pkg/sql/tests
Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/7e755dbdbcf0093f7ab792396ca99edebd740ee2/24123963428-1/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/905f7e3442bee845401cfbb4600fab828e348665/24123963428-1/\* old/

built with commit: 7e755dbdbcf0093f7ab792396ca99edebd740ee2

@cockroach-teamcity cockroach-teamcity added the X-perf-gain Microbenchmarks CI: Added if a performance gain is detected label Apr 8, 2026
@souravcrl
souravcrl force-pushed the fork/drop-provisioned-roles-parser branch from 7e755db to 88d4cc4 Compare April 8, 2026 08:44
@cockroach-teamcity

cockroach-teamcity commented Apr 8, 2026

Copy link
Copy Markdown
Member
⚪ Sysbench [SQL, 3node, oltp_read_write]
Metric Old Commit New Commit Delta Note
⚪ sec/op 10.22m ±1% 10.28m ±2% ~ p=0.305 n=15
⚪ allocs/op 8.122k ±1% 8.103k ±1% ~ p=0.406 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/88d4cc4/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/88d4cc420fe1674ec0763fd8f631b6fcd4694952/bin/pkg_sql_tests benchdiff/88d4cc4/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/88d4cc4/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=905f7e3 --new=88d4cc4 --memprofile ./pkg/sql/tests
⚪ Sysbench [KV, 3node, oltp_read_only]
Metric Old Commit New Commit Delta Note
⚪ sec/op 3.098m ±1% 3.078m ±1% -0.64% p=0.015 n=15
⚪ allocs/op 2.106k ±0% 2.106k ±0% ~ p=0.871 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/88d4cc4/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/88d4cc420fe1674ec0763fd8f631b6fcd4694952/bin/pkg_sql_tests benchdiff/88d4cc4/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/88d4cc4/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_read_only$ --old=905f7e3 --new=88d4cc4 --memprofile ./pkg/sql/tests
⚪ Sysbench [KV, 3node, oltp_write_only]
Metric Old Commit New Commit Delta Note
⚪ sec/op 2.922m ±0% 2.916m ±1% ~ p=0.838 n=15
⚪ allocs/op 4.204k ±0% 4.208k ±0% ~ p=0.056 n=15
Reproduce

benchdiff binaries:

mkdir -p benchdiff/88d4cc4/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/88d4cc420fe1674ec0763fd8f631b6fcd4694952/bin/pkg_sql_tests benchdiff/88d4cc4/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/88d4cc4/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/905f7e3/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/905f7e3442bee845401cfbb4600fab828e348665/bin/pkg_sql_tests benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/905f7e3/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

# NB: for best (most stable) results, also add a suitable `--benchtime` that
# results in ~1s to ~5s of benchmark runs. For example, if ops average ~3ms, a
# benchtime of `1000x` is appropriate.
#
# Some benchmarks (in particular BenchmarkSysbench) output additional memory
# profiles covering only the execution (excluding the setup/teardown) - those
# should be preferred for analysis since they more closely correspond to what's
# reported as B/op and alloc/op.
benchdiff --run=^BenchmarkSysbench/KV/3node/oltp_write_only$ --old=905f7e3 --new=88d4cc4 --memprofile ./pkg/sql/tests
Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/88d4cc420fe1674ec0763fd8f631b6fcd4694952/24126378736-1/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/905f7e3442bee845401cfbb4600fab828e348665/24126378736-1/\* old/

built with commit: 88d4cc420fe1674ec0763fd8f631b6fcd4694952

sanchit-CRL
sanchit-CRL reviewed Apr 8, 2026
View reviewed changes

@sanchit-CRL sanchit-CRL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description and commit message say LAST ACCESS TIME OLDER THAN but the actual grammar uses LAST LOGIN BEFORE .

No execution handler registered — statement will panic at runtime. DropProvisionedRoles has no case in planOpaque (pkg/sql/opaque.go). Running DROP PROVISIONED ROLES will hit errors.AssertionFailedf("planNode cannot be nil for %T", stmt) an internal error, not a graceful "unimplemented" message. Is this intended to be done in the next PR ?

Comment thread pkg/sql/parser/sql.y Outdated
@@ -6670,11 +6633,12 @@ discard_stmt:
// %Category: Group
// %Text:
// DROP DATABASE, DROP INDEX, DROP TABLE, DROP VIEW, DROP SEQUENCE,
// DROP USER, DROP ROLE, DROP TYPE
// DROP USER, DROP ROLE, DROP TYPE, DROP PROVISIONED ROLES,

@sanchit-CRL sanchit-CRL Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// DROP USER, DROP ROLE, DROP TYPE, DROP PROVISIONED ROLES,
// DROP USER, DROP ROLE, DROP TYPE, DROP PROVISIONED ROLES

@souravcrl souravcrl Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread pkg/sql/parser/testdata/drop_user
DROP PROVISIONED ROLES LIMIT 5
DROP PROVISIONED ROLES LIMIT (5) -- fully parenthesized
DROP PROVISIONED ROLES LIMIT _ -- literals removed
DROP PROVISIONED ROLES LIMIT 5 -- identifiers removed

@sanchit-CRL sanchit-CRL Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

DROP PROVISIONED ROLES WITH SOURCE = 'a', SOURCE = 'b'                                  
----                                                                                       
SOURCE option specified multiple times                                                     
                                        
error                                                                                      
DROP PROVISIONED ROLES WITH LAST LOGIN BEFORE '2025-01-01', LAST LOGIN BEFORE '2025-06-01'
----                                                                                       
LAST LOGIN BEFORE option specified multiple times```


these cases can also be added ?

@sanchit-CRL sanchit-CRL Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

DROP PROVISIONED ROLES WITH LAST LOGIN BEFORE '2025-01-01', SOURCE = 'ldap:ldap.example.com'                                             
----                                                                                       
DROP PROVISIONED ROLES WITH SOURCE = 'ldap:ldap.example.com', LAST LOGIN BEFORE '2025-01-01'```

reversed option order parse test can also be added ?

@souravcrl souravcrl Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@souravcrl @claude
sql: add parser grammar for DROP PROVISIONED ROLES
e622b04 
Extend the SQL parser grammar to support the DROP PROVISIONED ROLES
statement with optional WITH clauses and LIMIT:

  DROP PROVISIONED ROLES [WITH <options>] [LIMIT <n>]

Options (comma-separated):
  SOURCE = <string>
  LAST LOGIN BEFORE <expr>

Add `PROVISIONED` as an unreserved keyword. Define dedicated grammar
rules (`opt_with_drop_provisioned_roles_options`,
`drop_provisioned_roles_options_list`, `drop_provisioned_roles_option`)
and wire the new `drop_provisioned_roles_stmt` production into `drop_stmt`.

Add parse roundtrip tests covering all option combinations, including
error tests for duplicate option detection.

Fix a pre-existing format string bug in `combineExpr` and
`combineStringOrPlaceholderOptList` where `%` was used instead of `%s`,
producing garbled error messages for duplicate options.

Epic CRDB-52460

fixes: CRDB-52797

Release note: None

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@souravcrl
souravcrl force-pushed the fork/drop-provisioned-roles-parser branch from 88d4cc4 to e622b04 Compare April 16, 2026 10:33
@souravcrl @claude
sql: add reversed-order test case and fix comment for DROP PROVISIONE…
7fb5cb9 
…D ROLES

Add a parser test case for DROP PROVISIONED ROLES WITH options specified
in reversed order (LAST LOGIN BEFORE before SOURCE) to verify the
formatter normalizes option ordering. Also remove trailing comma from the
drop_stmt help text comment.

Release note: None
Epic: none

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sanchit-CRL
sanchit-CRL approved these changes May 4, 2026
View reviewed changes

@sanchit-CRL sanchit-CRL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@souravcrl

souravcrl commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as changes were reviewed and commited as part of #166980

@souravcrl souravcrl closed this May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sanchit-CRL sanchit-CRL sanchit-CRL approved these changes
@DrewKimball DrewKimball Awaiting requested review from DrewKimball DrewKimball is a code owner automatically assigned from cockroachdb/sql-queries-prs

Assignees

No one assigned

Labels

X-perf-check Microbenchmarks CI: Added to a PR if a performance regression is detected and should be checked X-perf-gain Microbenchmarks CI: Added if a performance gain is detected

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@souravcrl @cockroach-teamcity @sanchit-CRL