Opened 7 years ago

Closed 4 years ago

Last modified 4 years ago

#10320 closed New feature (fixed)

CursorDebugWrapper should allow using iterators/generators for executemany().

Reported by: MockSoul Owned by: nobody
Component: Database layer (models, ORM) Version: 1.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

executemany on cursors can have a little speedup on huge lists if developer sends iterators or generators as args. Thus, they will have no len. CursorDebugWrapper tries to determine length for only one reason: put a record about number of executed queries into connection.queries.

It's good idea probably to determine if args have no len -- dont put info about amount of executed queries into connection.queries.

Right now I can do connection.cursor().cursor.executemany(sql, iterator) for that purpose. I mean -- python db wrappers usually allow iterators in executemany's args. Another ugly way right now - catch TypeError and silently ignore. This is because django do len() after calling executemany on underlying cursor =). Non-pythonic, yeah?

Change History (9)

comment:1 Changed 7 years ago by MockSoul

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Easiest fixup:

  • django/db/backends/util.py

    === modified file 'django/db/backends/util.py'
     
    3232        finally:
    3333            stop = time()
    3434            self.db.queries.append({
    35                 'sql': '%s times: %s' % (len(param_list), sql),
     35                'sql': '%s times: %s' % (len(param_list) if hasattr(param_list, '__len__') else '??', sql),
    3636                'time': "%.3f" % (stop - start),
    3737            })

Because param_list may be iterator and already iterated iterator (hmm =)) -- there is no way to determine it's length after underlying executemany() cursor call. Thus, -- '??' is okay, imho :).

comment:2 Changed 7 years ago by MockSoul

  • Has patch set

comment:3 Changed 7 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:4 Changed 7 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:5 Changed 4 years ago by SmileyChris

  • Severity set to Normal
  • Type set to New feature

comment:6 Changed 4 years ago by aaugustin

  • Easy pickings unset
  • UI/UX unset

Strictly speaking, PEP 249 defines executemany(operation, seq_of_parameters), where seq_of_parameters is a sequence, and len is always available on sequences.

However, three out of four database backends support calling Cursor.executemany with an iterator as second argument:

  • The built-in sqlite3 module also allows using an iterator yielding parameters instead of a sequence.
  • It's clear from the source of pyscopg2, in cursor_type.c, that psycopg.Cursor.executemany accepts iterators, and if it receives a non-iterable, it actually converts it to an iterator -- if (!PyIter_Check(vars)) { vars = iter = PyObject_GetIter(vars); ... }.
  • MySQLdb.Cursor.executemany is implemented in Python, and the two possible code paths use for row in args, which clearly works with iterators.
  • Only cx_Oracle.Cursor.executemany fails to accept an iterator as argument: in Cursor.c, Cursor_ExecuteMany does numRows = PyList_GET_SIZE(listOfArguments);

So, it would be a good idea to support iterators in CursorDebugWrapper. Note that the performance argument is irrelevant for CursorDebugWrapper itself, since it's only used when DEBUG=True. The real reason for the fix is that CursorDebugWrapper shouldn't be more restrictive than regular cursors.

NB: django.db.backends.oracle.base.FormatStylePlaceholderCursor also relies on params not being a iterator, since it tries to access params[0]. But that could be fixed by adding something along the lines of if params is not None: params = list(params).

comment:7 Changed 4 years ago by aaugustin

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted

comment:8 Changed 4 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

In [17387]:

Fixed #10320 -- Made it possible to use executemany with iterators. Thanks MockSoul for the report.

comment:9 Changed 4 years ago by anonymous

Good to see you eventually close old tickets, thanks!!

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