Opened 3 years ago

Closed 3 years ago

Last modified 4 months ago

#22343 closed Bug (fixed)

select_for_update should not be executed outside of transactions

Reported by: Tim Graham Owned by: Shai Berger
Component: Database layer (models, ORM) Version: master
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Shai Berger)

Seeing these test failures after 0f9560855e5ed203b8c911c23237826e28a62a38:

$ ./runtests.py --settings=test_oracle select_for_update
Testing against Django installed in '/home/tim/code/django/django'
Creating test database for alias 'default'...
Creating test user...
Creating test database for alias 'other'...
Creating test user...
.EE...s
======================================================================
ERROR: test_for_update_sql_generated (select_for_update.tests.SelectForUpdateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/django/test/testcases.py", line 916, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/home/tim/code/django/tests/select_for_update/tests.py", line 80, in test_for_update_sql_generated
    list(Person.objects.all().select_for_update())
  File "/home/tim/code/django/django/db/models/query.py", line 141, in __iter__
    self._fetch_all()
  File "/home/tim/code/django/django/db/models/query.py", line 961, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/home/tim/code/django/django/db/models/query.py", line 265, in iterator
    for row in compiler.results_iter():
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 694, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 1150, in cursor_iter
    sentinel):
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 1149, in <lambda>
    for rows in iter((lambda: cursor.fetchmany(GET_ITERATOR_CHUNK_SIZE)),
  File "/home/tim/code/django/django/db/utils.py", line 101, in inner
    return func(*args, **kwargs)
  File "/home/tim/code/django/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/tim/code/django/django/db/utils.py", line 101, in inner
    return func(*args, **kwargs)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 905, in fetchmany
    return tuple(_rowfactory(r, self.cursor) for r in self.cursor.fetchmany(size))
DatabaseError: ORA-01002: fetch out of sequence


======================================================================
ERROR: test_for_update_sql_generated_nowait (select_for_update.tests.SelectForUpdateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/django/test/testcases.py", line 916, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/home/tim/code/django/tests/select_for_update/tests.py", line 89, in test_for_update_sql_generated_nowait
    list(Person.objects.all().select_for_update(nowait=True))
  File "/home/tim/code/django/django/db/models/query.py", line 141, in __iter__
    self._fetch_all()
  File "/home/tim/code/django/django/db/models/query.py", line 961, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/home/tim/code/django/django/db/models/query.py", line 265, in iterator
    for row in compiler.results_iter():
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 694, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 1150, in cursor_iter
    sentinel):
  File "/home/tim/code/django/django/db/models/sql/compiler.py", line 1149, in <lambda>
    for rows in iter((lambda: cursor.fetchmany(GET_ITERATOR_CHUNK_SIZE)),
  File "/home/tim/code/django/django/db/utils.py", line 101, in inner
    return func(*args, **kwargs)
  File "/home/tim/code/django/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/tim/code/django/django/db/utils.py", line 101, in inner
    return func(*args, **kwargs)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 905, in fetchmany
    return tuple(_rowfactory(r, self.cursor) for r in self.cursor.fetchmany(size))
DatabaseError: ORA-01002: fetch out of sequence


----------------------------------------------------------------------
Ran 7 tests in 3.987s

FAILED (errors=2, skipped=1)

But the problem is not really in the Oracle backend -- it is in allowing select_for_update() queries to run outside of transactions.

Change History (10)

comment:1 Changed 3 years ago by Shai Berger

Owner: changed from nobody to Shai Berger
Status: newassigned

Indeed. It seems like, in autocommit mode, the query execution is done in a (sub-)transaction separate from the fetches, which triggers this error when the selection is for update. I'm looking into it.

comment:2 Changed 3 years ago by Shai Berger

Description: modified (diff)
Summary: Two Oracle test failures since the removal of legacy transaction managementselect_for_update should not be executed outside of transactions

Patch here, reviews welcome (I think the code is simple enough, but documentation may need improvements).

I am not making this a PR, lest it be merged -- this needs backporting, and it's not trivial w.r.t release notes.

See also discussion on the developers mailing list.

comment:3 Changed 3 years ago by Tim Graham

Has patch: set
Triage Stage: AcceptedReady for checkin

PR looks good.

comment:4 Changed 3 years ago by Shai Berger <shai@…>

Resolution: fixed
Status: assignedclosed

In f095356ba2244f6ed48463bac0b46d3c3e671204:

Fixed #22343 -- Disallowed select_for_update in autocommit mode

The ticket was originally about two failing tests, which are
fixed by putting their queries in transactions.

Thanks Tim Graham for the report, Aymeric Augustin for the fix,
and Simon Charette, Tim Graham & Loïc Bistuer for review.

comment:5 Changed 3 years ago by Shai Berger <shai@…>

In 690a5984a3c0a706b8cc296bdcccd524b2079559:

[1.6.x] Fixed #22343 -- Disallowed select_for_update in autocommit mode

The ticket was originally about two failing tests, which are
fixed by putting their queries in transactions.

Thanks Tim Graham for the report, Aymeric Augustin for the fix,
and Simon Charette, Tim Graham & Loïc Bistuer for review.

Backport of b990df1d63 from master

comment:6 Changed 3 years ago by Shai Berger <shai@…>

In 3a9a4570efc933a50efec27bdd06af11877eeac6:

[1.7.x] Fixed #22343 -- Disallowed select_for_update in autocommit mode

The ticket was originally about two failing tests, which are
fixed by putting their queries in transactions.

Thanks Tim Graham for the report, Aymeric Augustin for the fix,
and Simon Charette, Tim Graham & Loïc Bistuer for review.

Backport of b990df1d63 from master

comment:7 Changed 3 years ago by Shai Berger <shai@…>

In 941f947bfd8d8f1a608634cf42f212b3a5a1933c:

[1.7.x] Documentation fixes for the select_for_update change.

Refs #22343; thanks Tim Graham for the fixes.

Backport of 59b1d30 from master

comment:8 Changed 3 years ago by Shai Berger <shai@…>

In 59b1d3098f393f0754b60ebb710450cba9891e6e:

Documentation fixes for the select_for_update change.

Refs #22343; thanks Tim Graham for the fixes.

comment:9 Changed 3 years ago by Shai Berger <shai@…>

In d5cef2a19c5a70766693264d08605f56d26e573d:

[1.6.x] Documentation fixes for the select_for_update change.

Refs #22343; thanks Tim Graham for the fixes.

comment:10 Changed 4 months ago by Simon Charette <charette.s@…>

In 29a3f8b:

Refs #22343 -- Corrected a test for missing select_for_update(nowait=True) support.

Note: See TracTickets for help on using tickets.
Back to Top