cockroachdb / cockroachdb/cockroach

sql/schemachanger: support placeholder arguments in declarative schema changer

Open
#142,256 6 comments 0 reactions 0 assignees View on GitHub
branch-master branch-release-25.1 C-bug T-sql-foundations
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

The declarative schema changer does not have good support for placeholders in SQL statements.

**To Reproduce**

Running on 8c5af013aa7

```
root@localhost:26257/defaultdb> create table foo (a int);
CREATE TABLE

root@localhost:26257/defaultdb> set use_declarative_schema_changer = unsafe_always;
SET

root@localhost:26257/defaultdb> prepare q as ALTER TABLE foo CONFIGURE ZONE USING gc.ttlseconds = 1, range_max_bytes = $1, range_min_bytes = 1<<10;
ERROR: internal error: building declarative schema change targets for CONFIGURE ZONE: expected *DInt, found *tree.Placeholder
SQLSTATE: XX000
DETAIL: stack trace:
pkg/sql/sem/tree/datum.go:691: MustBeDInt()
pkg/sql/catalog/zone/zones.go:65: func2()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go:388: func1()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go:462: func1()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go:465: loadSettingsToZoneConfigs()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go:1183: prepareZoneConfig()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/table_zone_config.go:201: applyZoneConfig()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/configure_zone.go:70: SetZoneConfig()
GOROOT/src/reflect/value.go:581: call()
GOROOT/src/reflect/value.go:365: Call()
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go:192: Process()
pkg/sql/schemachanger/scbuild/build.go:102: Build()
pkg/sql/schema_change_plan_node.go:70: SchemaChange()
pkg/sql/opaque.go:52: buildOpaque()
pkg/sql/opt/optbuilder/opaque.go:65: tryBuildOpaque()
pkg/sql/opt/optbuilder/builder.go:506: buildStmt()
pkg/sql/opt/optbuilder/builder.go:323: buildStmtAtRootWithScope()
pkg/sql/opt/optbuilder/builder.go:304: buildStmtAtRoot()
pkg/sql/opt/optbuilder/builder.go:283: Build()
pkg/sql/plan_opt.go:494: buildReusableMemo()
pkg/sql/plan_opt.go:173: prepareUsingOptimizer()
pkg/sql/conn_executor_prepare.go:339: populatePrepared()
pkg/sql/conn_executor_prepare.go:279: func2()
pkg/sql/conn_executor_prepare.go:284: prepare()
pkg/sql/conn_executor_prepare.go:117: addPreparedStmt()
pkg/sql/conn_executor_exec.go:895: execStmtInOpenState()
pkg/sql/conn_executor_exec.go:165: func2()
pkg/sql/conn_executor_exec.go:4428: execWithProfiling()
pkg/sql/conn_executor_exec.go:164: execStmt()
pkg/sql/conn_executor.go:2424: func1()
pkg/sql/conn_executor.go:2429: execCmd()
pkg/sql/conn_executor.go:2346: run()
```

Here is a different example of a statement that can be prepared, but then fails at execution time.
```
root@localhost:26257/defaultdb> create table foo (a int);
CREATE TABLE

root@localhost:26257/defaultdb> set use_declarative_schema_changer = unsafe_always;
SET

root@localhost:26257/defaultdb> prepare q as alter table foo add column b int default $1:::int;
PREPARE

Time: 1ms total (execution 0ms / network 0ms)

root@localhost:26257/defaultdb> execute q(3::int);
ERROR: internal error: no type for placeholder $1
SQLSTATE: XX000
DETAIL: stack trace:
pkg/sql/execute.go:38: fillInPlaceholders()
pkg/sql/conn_executor_exec.go:523: execStmtInOpenState()
pkg/sql/conn_executor_exec.go:165: func2()
pkg/sql/conn_executor_exec.go:4428: execWithProfiling()
pkg/sql/conn_executor_exec.go:164: execStmt()
pkg/sql/conn_executor.go:2424: func1()
pkg/sql/conn_executor.go:2429: execCmd()
pkg/sql/conn_executor.go:2346: run()
pkg/sql/conn_executor.go:1017: ServeConn()
pkg/sql/pgwire/conn.go:252: processCommands()
pkg/sql/pgwire/server.go:1197: func4()
src/runtime/asm_arm64.s:1223: goexit()
```

If `use_declarative_schema_changer` were set to its default value, then the above statements would instead fallback to the legacy schema changer.

**Additional context:**

This was previously reported in https://github.com/cockroachdb/cockroach/issues/126288. That issue was closed but the fix for it did not fully work.

Jira issue: CRDB-48241

Epic CRDB-60938

Contributor guide

Open the contributing guide

Research direction

Reproduce the two prepared-statement examples with the declarative schema changer enabled. Start with pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go and the related stack-trace entry points, then trace placeholder handling through pkg/sql/execute.go and pkg/sql/conn_executor_exec.go. Done means the shown CONFIGURE ZONE and ADD COLUMN statements support placeholder arguments without internal errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.