Alex Lord added the comment:

There was a bunch of things wrong with that patch.

In addition to the issues you brought up in the review I was mixing up what the 
actual problem is

REPLACE INTO is an alias for INSERT OR REPLACE. INSERT OR REPLACE was correctly 
setting the lastrowid values but REPLACE INTO was not setting the last rowid 
value. I changed the documentation modifications to reflect this.

In addition I cleaned up the unit tests. The unit tests were kind of a mess 
because I was trying to figure out what the problem was and never refactored 
after getting a reproduction.

 I at first went down the path of making the tests use a for loop like you 
suggested

    for statement in ["INSERT OR REPLACE", "REPLACE"]:
        sql = "{} INTO test(id, unique_test) VALUES (?, ?)".format(
            statement)
        self.cu.execute(sql, (1, "foo"))
        self.assertEqual(self.cu.lastrowid, 1)
        self.cu.execute(sql, (1, "foo"))$
        self.assertEqual(self.cu.lastrowid, 1) 

Which I don't think is as nice as a cleaned up unrolled version

    self.cu.execute("INSERT OR REPLACE INTO test(id, unique_test) VALUES (?, 
?)", (1, "bar",))
    self.assertEqual(self.cu.lastrowid, 1)
    self.cu.execute("INSERT OR REPLACE INTO test(id, unique_test) VALUES (?, 
?)", (1, "bar",))
    self.assertEqual(self.cu.lastrowid, 1)
    self.cu.execute("REPLACE INTO test(id, unique_test) VALUES (?, ?)", (1, 
"bar",))
    self.assertEqual(self.cu.lastrowid, 1)

I've created a patch that fixes all of the issues brought up in the code review 
and is just generally much cleaner.

----------
Added file: http://bugs.python.org/file39411/sqlite_after_review_1.patch

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue16864>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to