Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: prepared statements against tables with enum types not invalidated when enum changes #70378

Closed
jordanlewis opened this issue Sep 17, 2021 · 4 comments · Fixed by #71632
Closed
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@jordanlewis
Copy link
Member

jordanlewis commented Sep 17, 2021

The following sequence of commands should work properly, but instead return an error. The issue seems to be that the prepared statement doesn't notice that the table that it's referencing has an ENUM type that got modified, causing it to refresh its referenced descriptors.

create type t as enum('a');
create table tab (t t);
prepare x as insert into tab values($1);
execute x('a');
alter type t add value 'b';
execute x('b');

ERROR: invalid input value for enum t: "b"
SQLSTATE: 22P02
[email protected]:26257/defaultdb>

I'd expect that this should work fine, or at worst, the error should be about the prepared statement being out of date or something.

Epic CRDB-8948

@jordanlewis jordanlewis added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Sep 17, 2021
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Sep 17, 2021
@ajwerner
Copy link
Contributor

The problem here is sort of known and relates to #64140 and a bit in #65620.

The issue is that we don't track what leases we use in bind the way we do in prepare and, even if we did, we don't retain the initial value and thus could not re-parse the datum. In terms of planning, the memo tracks the versions it uses and then, upon execution, calls IsStale() which re-acquires leases. Ideally in bind we'd do something similar and then in execution, after opening a transaction, re-bind the datums and maybe re-prepare or something. I'm inclined to move this to the @cockroachdb/sql-experience board as this is about state handling in BIND and that's where we're tracking #64140. @jordanlewis and @rafiss does that sound reasonable to you?

@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Sep 21, 2021
@rafiss rafiss self-assigned this Oct 11, 2021
@rafiss
Copy link
Collaborator

rafiss commented Oct 15, 2021

postgres seems to delete the prepared statement:

rafiss@127:postgres> create type tp as enum('a');
CREATE TYPE
Time: 0.030s
rafiss@127:postgres> create table tab (a tp);
CREATE TABLE
Time: 0.005s
rafiss@127:postgres> prepare x as insert into tab values($1);
PREPARE
Time: 0.003s
rafiss@127:postgres> execute x('a');
INSERT 0 1
Time: 0.003s
rafiss@127:postgres> alter type tp add value 'b';
You're about to run a destructive command.
Do you want to proceed? (y/n): y
Your call!
ALTER TYPE
Time: 0.002s
rafiss@127:postgres> execute x('b');
prepared statement "x" does not exist

@otan
Copy link
Contributor

otan commented Oct 15, 2021

that's smart

@rafiss
Copy link
Collaborator

rafiss commented Oct 15, 2021

hm, looks like wire-level prepare and the PREPARE command are different. In Postgres, this works:

	db.Exec("DROP TABLE IF EXISTS tb")
	db.Exec("DROP TYPE IF EXISTS te")
	db.Exec("CREATE TYPE te AS ENUM ('a')")
	db.Exec("CREATE TABLE tb (a te)")
	stmt, err := db.Prepare("INSERT INTO tb VALUES ($1)")
	require.NoError(t, err)
	_, err = stmt.Exec("a")
	require.NoError(t, err)
	db.Exec("ALTER TYPE te ADD VALUE 'b'")
	_, err = stmt.Exec("b")
	require.NoError(t, err)

	var c int
	err = db.QueryRow("SELECT COUNT(8) FROM tb").Scan(&c)
	require.NoError(t, err)
	require.Equal(t, 2, c)

but in CRDB that gives error in argument for $1: enum value "b" is not yet public.


also i don't know what i did before, but the execute x('b') example above actually does work fine in Postgres and just inserts the value 'b'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants