Python update SQLite DB Values

Clash Royale CLAN TAG#URR8PPP
$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:
- Is it really necessary to make the select or could I use the present Data in any other way?
- 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))
python performance beginner sqlite
$endgroup$
add a comment |
$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:
- Is it really necessary to make the select or could I use the present Data in any other way?
- 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))
python performance beginner sqlite
$endgroup$
add a comment |
$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:
- Is it really necessary to make the select or could I use the present Data in any other way?
- 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))
python performance beginner sqlite
$endgroup$
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:
- Is it really necessary to make the select or could I use the present Data in any other way?
- 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))
python performance beginner sqlite
python performance beginner sqlite
asked Feb 12 at 7:26
K-DoeK-Doe
955
955
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
$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$
add a comment |
$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 theconnectcall from thewithstatement as shown in the docs and wrap theconnectcall in awith contextlib.closing(..) as connif 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. aswith conn.transaction() as t: ….
$endgroup$
– Konrad Rudolph
Feb 12 at 17:22
$begingroup$
Well, that is an other argument to separate thewith conn:call from theconnectone (as it would be nearly useless to dowith 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
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f213288%2fpython-update-sqlite-db-values%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$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$
add a comment |
$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$
add a comment |
$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$
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.
answered Feb 12 at 9:20
Mathias EttingerMathias Ettinger
25.1k33184
25.1k33184
add a comment |
add a comment |
$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 theconnectcall from thewithstatement as shown in the docs and wrap theconnectcall in awith contextlib.closing(..) as connif 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. aswith conn.transaction() as t: ….
$endgroup$
– Konrad Rudolph
Feb 12 at 17:22
$begingroup$
Well, that is an other argument to separate thewith conn:call from theconnectone (as it would be nearly useless to dowith 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
add a comment |
$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 theconnectcall from thewithstatement as shown in the docs and wrap theconnectcall in awith contextlib.closing(..) as connif 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. aswith conn.transaction() as t: ….
$endgroup$
– Konrad Rudolph
Feb 12 at 17:22
$begingroup$
Well, that is an other argument to separate thewith conn:call from theconnectone (as it would be nearly useless to dowith 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
add a comment |
$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$
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))
edited Feb 12 at 14:13
answered Feb 12 at 14:08
Konrad RudolphKonrad Rudolph
4,9811631
4,9811631
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 theconnectcall from thewithstatement as shown in the docs and wrap theconnectcall in awith contextlib.closing(..) as connif 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. aswith conn.transaction() as t: ….
$endgroup$
– Konrad Rudolph
Feb 12 at 17:22
$begingroup$
Well, that is an other argument to separate thewith conn:call from theconnectone (as it would be nearly useless to dowith 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
add a comment |
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 theconnectcall from thewithstatement as shown in the docs and wrap theconnectcall in awith contextlib.closing(..) as connif 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. aswith conn.transaction() as t: ….
$endgroup$
– Konrad Rudolph
Feb 12 at 17:22
$begingroup$
Well, that is an other argument to separate thewith conn:call from theconnectone (as it would be nearly useless to dowith 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
1
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$
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$
@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$
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$
@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
$begingroup$
Thank you for your well written answere! I will learn alot from it.
$endgroup$
– K-Doe
Feb 13 at 6:32
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f213288%2fpython-update-sqlite-db-values%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown