Code

Opened 5 years ago

Closed 2 years ago

Last modified 2 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?

Attachments (0)

Change History (9)

comment:1 Changed 5 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 5 years ago by MockSoul

  • Has patch set

comment:3 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:4 Changed 5 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:5 Changed 3 years ago by SmileyChris

  • Severity set to Normal
  • Type set to New feature

comment:6 Changed 2 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 2 years ago by aaugustin

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

comment:8 Changed 2 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 2 years ago by anonymous

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.