5
\$\begingroup\$

At the moment I try to get used to SQLite and Python. I started with insert, select and so on. But now i want to get a bit further. Now I have a DB with 200 rows for testing and in want to update two values of each row in this case called "Test2" and "Test3". My problem is that those two values should be the value that they have +200 and +400 so i have to make an select before updating them (at least with my knowledge).

I added a simple time measurement to my code to see if i will be able to make my code more efficient and faster!

At the moment it takes 0.7344 seconds for me to edit 200 rows of data. Which is pretty slow! Especially if i think to do it for bigger DB's.

Things I want to achieve:

  1. Is it really necessary to make the select or could I use the present Data in any other way?
  2. Make things faster! Is it possible to edit multiple rows at the same time? Like parallelized tasks?

Any other tips are welcome, as i try to improve myself!

Here is my code so far:

import sqlite3
import time

def database_test():
    # Open Database
    conn = sqlite3.connect('SQLite_Test.db')
    c = conn.cursor()

    i = 0

    for i in range(200):
        c.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?',  (i,))

        DB_Values = []        
        DB_Values = c.fetchone()

        Value1 = DB_Values[0]+200
        Value2 = DB_Values[1]+400

        c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
        i += 1  

    # Save (commit) the changes
    conn.commit() 


start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time),4))
\$\endgroup\$
8
\$\begingroup\$

SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.

This means that you can simplify your whole loop into a single statement:

query = '''
    UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
    WHERE Test1 >= 0 AND Test1 < 200
'''
c.execute(query, (200, 400))

Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.

\$\endgroup\$
5
\$\begingroup\$

Mathias’ answer is the correct way of solving your problem but since this is a code review site, let’s look at the rest of your code.

First off, your code style is fundamentally tidy and clear, so that’s very good. Now for some detailed feedback:

    # Open Database

Comments such as this one are at best unnecessary and at worst harmful: the comment provides no additional information compared to the code, and at worst they are misleading. Case in point, what happens if the file SQLite_Test.db doesn’t exist yet? Your comment gives no indication — it implies that the file must exist. But SQLite will actually happily create it for you if it doesn’t exist yet.

Jessica Baker has written a very good article explaining how to write comments well. I strongly urge anyone to read it in its entirety.

    c = conn.cursor()

Your code doesn’t need database cursors: Although the subsequent code uses c, it could use conn instead.

    i = 0

This initialisation is redundant: range(…) does it for you.

    for i in range(200):

Why 200? Avoid hard-coding “magic” numbers. If this is the size of your database table, don’t put the number in code, compute it from the database. If the number is arbitrary (for testing, say), assign it to a variable stating thus.

        DB_Values = []        
        DB_Values = c.fetchone()

The first line here sets DB_Values to an empty list; the second line immediately overrides that value. The first lines is therefore unnecessary.

Apart from this, you should respect PEP 8 naming conventions. Lastly, db_values isn’t a very clear name: db_row might be more expressive.

        Value1 = DB_Values[0]+200
        Value2 = DB_Values[1]+400

Same comment regarding names, and same comment regarding magic numbers.

        c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
        i += 1  

You might think that it’s necessary to increment i to progress the loop but this isn’t the case. The increment of i fulfils no purpose, since it will be overridden by the for loop anyway.

    # Save (commit) the changes

Same as above: this comment isn’t helpful, since it just repeats what the code says.

    conn.commit()

If there was an exception in the preceding code, this commit will never be executed. You may have intended this; but chances are, you didn’t. It’s therefore best practice to avoid implicit resource cleanup (including database commits), and to use Python’s with statement instead.


Taking this in account, here’s a rewritten version of your code:

import sqlite3
import time

TEST_NUM_ROWS = 200
TEST2_INC = 200
TEST3_INC = 400

def database_test():
    with sqlite3.connect('SQLite_Test.db') as conn:
        for i in range(TEST_NUM_ROWS):
            row = conn.execute(
                'SELECT Test2, Test3 FROM Test WHERE Test1 = ?',
                (i,)
            ).fetchone()
            new_row = (row[0] + TEST2_INC, row[1] + TEST3_INC)
            conn.execute(
                'UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1 = ?',
                new_row + (i,)
            )


start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time), 4))
\$\endgroup\$
  • 1
    \$\begingroup\$ I like this answer except how you use the context manager. There always seem to be some level of confusion about what the context manager on a DB connection is doing. To avoid people thinking that the connection will be closed after executing the with block, I usually separate the connect call from the with statement as shown in the docs and wrap the connect call in a with contextlib.closing(..) as conn if need be. \$\endgroup\$ – Mathias Ettinger Feb 12 at 16:30
  • \$\begingroup\$ @MathiasEttinger I understand your concern but separating the initialisation from the context manager doesn’t fix the issue: every competent Python programmer knows that the resulting code is in fact equivalent. Rather, this needs to be fixed at the API level in sqlite3; e.g. as with conn.transaction() as t: …. \$\endgroup\$ – Konrad Rudolph Feb 12 at 17:22
  • \$\begingroup\$ Well, that is an other argument to separate the with conn: call from the connect one (as it would be nearly useless to do with sqlite3.connect(...).transaction()) ;) \$\endgroup\$ – Mathias Ettinger Feb 12 at 18:24
  • \$\begingroup\$ @MathiasEttinger If that API existed I’d agree, yes. \$\endgroup\$ – Konrad Rudolph Feb 12 at 18:40
  • \$\begingroup\$ Thank you for your well written answere! I will learn alot from it. \$\endgroup\$ – K-Doe Feb 13 at 6:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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