6
\$\begingroup\$

I have a bit of code to insert some values in a table. It seems a bit clunky - is there a neater way?

words is a list of dictionaries, and the entries contain other fields which I don't want to insert.

# add the list of words to the words table
time_stamp = datetime.datetime.now().isoformat()
sql = "INSERT INTO words (word, user_id, game_id, score, time_stamp) VALUES "
vals = []
for word in words:
    vals.append("('%s', %s, %s, %s, '%s')" %
                (word['str'], user_id, game_id, word['score'], time_stamp))
cur.execute(sql + ",".join(vals))
\$\endgroup\$
1
  • \$\begingroup\$ If user_id and game_id came from the user/UI, would you assemble the sql string that way? \$\endgroup\$ Jun 3, 2014 at 16:26

2 Answers 2

7
\$\begingroup\$

There is a neater, more Pythonic way of doing this using the str.format function. However, it is not the way you should do it.

Currently, the way you are building your query, you are vulnerable to SQL injection. This is the point @Mat's Mug was getting at in his comment. What if a malicious user decides to give the following as a user ID:

user_id = "1,1,99,'today');DROP TABLES;INSERT INTO words (word, user_id, game_id, score, time_stamp) VALUES ('Goodbye', 1"

If this happens, a disaster beyond your imagination will occur*!

To prevent this, the cursor from the MySQLdb module (which I assume you are using) allows you to pass in a tuple or dict of the parameters to the execute function:

time_stamp = datetime.datetime.now().isoformat()
sql = "INSERT INTO words (word, user_id, game_id, score, time_stamp) VALUES {}"

# Create the query
sql = sql.format(','.join(["('%s', %s, %s, %s, '%s')" for _ in words]))

# Create the parameter tuple.
params = ()
for word in words:
    params += (word['str'], user_id, game_id, word['score'], time_stamp,)

# Sending a tuple is easiest in this situation
cur.execute(sql, params)

If you want to look at another database solution for Python, take a look at SQLAlchemy. Its a pretty powerful framework that makes working with databases much simpler.


*=Sorry for the Phantom of the Opera quote. I'm feeling oddly dramatic right now.

Update

To answer your question about removing the for-loop, the best you can hope for is to use a list comprehension. In many situations, list comprehensions are typically the Pythonic way of iterating over a collection. However, in your situation, this probably is not the case.

The comprehension for your current code would look like this:

','.join([str(tuple([word['str'], user_id, game_id, word['score'], time_stamp]))
          for word in words])

This is really only effective creating the end to your query. Unfortunately, in addition to being hard to read, it is still vulnerable to SQL injection.

In your case, I think a for-loop would be the cleanest way to implement what you want.

\$\endgroup\$
2
  • \$\begingroup\$ Sorry, I'm well aware that it's not secure - I removed the escaping code to highlight the issue which is: can I remove the for loop somehow? Is there a more idiomatic way to build the params [string|tuple]? \$\endgroup\$ Jun 4, 2014 at 10:20
  • \$\begingroup\$ I have added a section addressing your question. \$\endgroup\$ Jun 4, 2014 at 12:44
0
\$\begingroup\$

The even better way to insert many records, is to use cursor.executemany() instead.

You did not state what API you use, but AFAIK at least the libraries commonly used with PostgreSQL, MySQL and SQLite all support this (maybe you need to adapt the placeholders). I didn't look at others...

Example:

# add the list of words to the words table
time_stamp = datetime.datetime.now().isoformat()
vals = ((word['str'], user_id, game_id, word['score'], time_stamp)
        for word in words)
sql = """\
INSERT INTO words (word, user_id, game_id, score, time_stamp)
VALUES (%s, %s, %s, %s, %s)
"""
cur.executemany(sql, vals)

Depending on the specific implementation, there may be more efficient ways to do this (prepared statements), and you may or may not be able to use a generator instead of a list to pass the values.

Note that you should not need to add the ticks '%s' for placeholders of string values (use this instead: %s). The value adapters of the library you're using should add these and also properly pad them with appropriate additional escaping mechanisms.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.