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: DELETE FROM is broken for system.jobs table #40890

Closed
spaskob opened this issue Sep 18, 2019 · 6 comments · Fixed by #40898
Closed

sql: DELETE FROM is broken for system.jobs table #40890

spaskob opened this issue Sep 18, 2019 · 6 comments · Fixed by #40898
Assignees
Labels
A-disaster-recovery A-jobs A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@spaskob
Copy link
Contributor

spaskob commented Sep 18, 2019

Describe the problem

DELETE FROM system.jobs WHERE id IN (x_1, x_2, ..., x_n) only deletes 1 row even if all the x_i are in the table. Always only the last element (x_n) is deleted.

Interestingly, the faulty behavior only holds for system.jobs. I was not able to reproduce it for user defined tables or for other system tables (tested with system.zones).

To Reproduce*

  1. Run demo cockroach demo
  2. See ids from jobs table
[email protected]:49862/movr> select id from system.jobs;
          id
+--------------------+
  487489753862537217
  487489754741211137
  487489755233714177
 487489755702657025 
 <snip>

(10 rows)
  1. Try to delete the first 3 rows
[email protected]:49862/movr> delete from system.jobs where id IN (487489753862537217, 487489754741211137, 487489755233714177);
DELETE 1
  1. Only 1 row actually gets deleted (note that is always the last row in the IN list
[email protected]:49862/movr> select id from system.jobs;
          id
+--------------------+
  487489753862537217
  487489754741211137
  487489755702657025
  <snip>
(9 rows)

Expected behavior
Delete should delete all matching rows as it does for other tables.

Additional data / screenshots
None

Environment:

  • CockroachDB version [e.g. 2.0.x]
  • Server OS: darwin
  • Client app: demo

Additional context
This leaves lots of finished jobs rows in the system.jobs table.
See #40563

@spaskob spaskob added A-disaster-recovery A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. A-jobs labels Sep 18, 2019
@jordanlewis
Copy link
Member

Thanks @spaskob - this appears to be an issue with the creation of constraint spans in some cases.

Check out the explain:

[email protected]:58310/movr> explain delete from system.jobs where id IN (487496322573697025, 487496322991783937, 487496323417014273);
       tree      |    field    |                 description
+----------------+-------------+---------------------------------------------+
                 | distributed | false
                 | vectorized  | false
  count          |             |
   └── delete    |             |
        │        | from        | jobs
        │        | strategy    | deleter
        └── scan |             |
                 | table       | jobs@primary
                 | spans       | /487496323417014273/0-/487496323417014273/1
                 | parallel    |
(10 rows)

Time: 885µs

[email protected]:58310/movr> explain(opt) delete from system.jobs where id IN (487496322573697025, 487496322991783937, 487496323417014273);
                                                                              text
+---------------------------------------------------------------------------------------------------------------------------------------------------------------+
  delete system.public.jobs
   └── scan system.public.jobs
        └── constraint: /6: [/487496322573697025 - /487496322573697025] [/487496322991783937 - /487496322991783937] [/487496323417014273 - /487496323417014273]
(3 rows)

Time: 1.075ms

In the explain, you can see that there's only a single issued span for the delete! That is bad - it means that we're only trying to delete 1 row for some reason.

But, in the explain(opt), all spans are listed as expected. So, something is going wrong when translated the opt constraints into physical spans...

@jordanlewis
Copy link
Member

I've tracked the bug down to a method called spansFromConstraint, which translates an optimizer constraint (constraint.Constraint) into physical spans (roachpb.Spans).

Before that method, the optimizer constraints are as expected:

Constraint:  /6: [/487496322573697025 - /487496322573697025] [/487496322991783937 - /487496322991783937] [/487496323417014273 - /487496323417014273]

After that method, the physical spans produced:

[/Table/15/1/487496323417014273/{0-1}]

@jordanlewis
Copy link
Member

I think it has to do with tables with column families. As such, I thought it was introduced by #30744, but that PR is present in 19.1 and this problem doesn't repro in 19.1. Here's a tight repro on master without system tables:

[email protected]:58506/defaultdb> create table a (a int primary key, b int, family(a), family(b));
CREATE TABLE

Time: 2.222ms

[email protected]:58506/defaultdb> explain(opt) select a from a where a in (1,4);
                    text
+------------------------------------------+
  scan a
   └── constraint: /1: [/1 - /1] [/4 - /4]
(2 rows)

Time: 1.986ms

[email protected]:58506/defaultdb> explain select a from a where a in (1,4);
  tree |    field    | description
+------+-------------+-------------+
       | distributed | false
       | vectorized  | false
  scan |             |
       | table       | a@primary
       | spans       | /4/0-/4/1
       | parallel    |
(6 rows)

Time: 549µs

@kenliu
Copy link

kenliu commented Sep 18, 2019

should we consider this a release blocker for 19.2?

@kenliu kenliu added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Sep 18, 2019
@jordanlewis
Copy link
Member

Definitely.

@jordanlewis
Copy link
Member

Okay, looks like this was introduced by #38301.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-disaster-recovery A-jobs A-sql-mutations Mutation statements: UPDATE/INSERT/UPSERT/DELETE. 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.

3 participants