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

multiple schema changes on same request don't work on SQL over http api #86332

Closed
maryliag opened this issue Aug 17, 2022 · 0 comments · Fixed by #86433
Closed

multiple schema changes on same request don't work on SQL over http api #86332

maryliag opened this issue Aug 17, 2022 · 0 comments · Fixed by #86433
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@maryliag
Copy link
Contributor

maryliag commented Aug 17, 2022

When sending a request with multiple schema changes on the SQL over HTTP api, it doesn't actually make all the changes.

To test: create table CREATE TABLE t1 (k INT, i INT, f FLOAT, s STRING);
If I make a request with a single query, such as CREATE INDEX ON t1 (i) STORING (k);, it works.

If I instead create a index (on cli or the api) CREATE INDEX ON t1 (i); (which creates the idx t1_i_idx), and then I make a call to the api passing two statements:
CREATE INDEX IF NOT EXISTS t1_i__idx ON t1 (i) STORING (k);
DROP INDEX t1@t1_i_idx;
the api return no errors, but checking the table I can see that the t1_i_idx was deleted, but t1_i__idx was not created. If I then try to run the same statement CREATE INDEX IF NOT EXISTS t1_i__idx ON t1 (i) STORING (k); directly on cli, it doesn't return any errors, but also doesn't create the index.

Trying to drop t1 just hangs after that.

> select crdb_internal.pb_to_json('desc', descriptor) from system.descriptor where id = 't1'::regclass;

{"table": {"columns": [{"id": 1, "name": "k", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 2, "name": "i", "nullable": true, "type": {"family": "IntFamily", "oid": 20, "width": 64}}, {"id": 3, "name": "f", "nullable": true, "type": {"family": "FloatFamily", "oid": 701, "width": 64}}, {"id": 4, "name": "s", "nullable": true, "type": {"family": "StringFamily", "oid": 25}}, {"defaultExpr": "unique_rowid()", "hidden": true, "id": 5, "name": "rowid", "type": {"family": "IntFamily", "oid": 20, "width": 64}}], "createAsOfTime": {"wallTime": "1660766203622664000"}, "families": [{"columnIds": [1, 2, 3, 4, 5], "columnNames": ["k", "i", "f", "s", "rowid"], "name": "primary"}], "formatVersion": 3, "id": 112, "modificationTime": {}, "mutationJobs": [{"jobId": "788713496337022977", "mutationId": 2}, {"jobId": "788713496363499521", "mutationId": 3}], "mutations": [{"direction": "ADD", "index": {"createdAtNanos": "1660766657425427000", "createdExplicitly": true, "foreignKey": {}, "geoConfig": {}, "id": 4, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [2], "keyColumnNames": ["i"], "keySuffixColumnIds": [5], "name": "t1_i__idx", "partitioning": {}, "sharded": {}, "storeColumnIds": [1], "storeColumnNames": ["k"], "version": 3}, "mutationId": 2, "state": "BACKFILLING"}, {"direction": "ADD", "index": {"createdAtNanos": "1660766657425427000", "createdExplicitly": true, "foreignKey": {}, "geoConfig": {}, "id": 5, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [2], "keyColumnNames": ["i"], "keySuffixColumnIds": [5], "name": "t1_i_crdb_internal_dpe_idx", "partitioning": {}, "sharded": {}, "storeColumnIds": [1], "storeColumnNames": ["k"], "useDeletePreservingEncoding": true, "version": 3}, "mutationId": 2, "state": "DELETE_ONLY"}, {"direction": "DROP", "index": {"createdAtNanos": "1660766232871093000", "createdExplicitly": true, "foreignKey": {}, "geoConfig": {}, "id": 2, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [2], "keyColumnNames": ["i"], "keySuffixColumnIds": [5], "name": "t1_i_idx", "partitioning": {}, "sharded": {}, "version": 3}, "mutationId": 3, "state": "DELETE_AND_WRITE_ONLY"}], "name": "t1", "nextColumnId": 6, "nextConstraintId": 2, "nextFamilyId": 1, "nextIndexId": 6, "nextMutationId": 4, "parentId": 104, "primaryIndex": {"constraintId": 1, "createdAtNanos": "1660766203622649000", "encodingType": 1, "foreignKey": {}, "geoConfig": {}, "id": 1, "interleave": {}, "keyColumnDirections": ["ASC"], "keyColumnIds": [5], "keyColumnNames": ["rowid"], "name": "t1_pkey", "partitioning": {}, "sharded": {}, "storeColumnIds": [1, 2, 3, 4], "storeColumnNames": ["k", "i", "f", "s"], "unique": true, "version": 4}, "privileges": {"ownerProto": "root", "users": [{"privileges": 2, "userProto": "admin", "withGrantOption": 2}, {"privileges": 2, "userProto": "root", "withGrantOption": 2}], "version": 2}, "replacementOf": {"time": {}}, "unexposedParentSchemaId": 105, "version": "9"}}

Jira issue: CRDB-18702

@maryliag maryliag added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 17, 2022
ajwerner added a commit to ajwerner/cockroach that referenced this issue Aug 18, 2022
We need to construct the internal executor in the context of the transaction
so that we can make sure that its side-effects are properly managed. Without
this change, we'd be throwing away all of the extraTxnState between each
statement. We'd fail to create the jobs (which we defer to the end of the
transaction), and we'd fail to run those jobs and check for errors. We'd
also fail to validate the two-version invariant or wait for one version.

Fixes cockroachdb#86332

Release justification: Fixes critical bugs in new functionality.

Release note: None
ajwerner added a commit to ajwerner/cockroach that referenced this issue Aug 19, 2022
We need to construct the internal executor in the context of the transaction
so that we can make sure that its side-effects are properly managed. Without
this change, we'd be throwing away all of the extraTxnState between each
statement. We'd fail to create the jobs (which we defer to the end of the
transaction), and we'd fail to run those jobs and check for errors. We'd
also fail to validate the two-version invariant or wait for one version.

Fixes cockroachdb#86332

Release justification: Fixes critical bugs in new functionality.

Release note: None
craig bot pushed a commit that referenced this issue Aug 20, 2022
86433: server: proper transaction state management in sql-over-http  r=ajwerner a=ajwerner

First 4 commits are #86427.
Next commit is #86461.

We need to construct the internal executor in the context of the transaction
so that we can make sure that its side-effects are properly managed. Without
this change, we'd be throwing away all of the extraTxnState between each
statement. We'd fail to create the jobs (which we defer to the end of the
transaction), and we'd fail to run those jobs and check for errors. We'd
also fail to validate the two-version invariant or wait for one version.

Fixes #86332

Release justification: Fixes critical bugs in new functionality.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
@craig craig bot closed this as completed in 4812378 Aug 20, 2022
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant