HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Check existence of a row then update a column

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
updateexistencecolumnthencheckrow

Problem

not too experienced with SQL, wondering if I can make this better... I am trying to check if a row exists and then add one to the frequency column if it does, if it does not exist it should return false...

The table is indexed by the column in the first select statement

I know about the risks of using formatting with public facing code, but this is for processing on a local machine only

def check_word(self, word_name):
        self.c.execute('SELECT * FROM {tn} WHERE {cn} = """{wn}"""'.format(tn=self.table1, cn=self.column1, wn=word_name))
        exist = self.c.fetchall()
        if exist:
            new_freq = exist[0][2] + 1
            self.c.execute(
            'UPDATE {tn} SET {c2n}={en} WHERE {c1n}="""{word}"""'.format(tn=self.table1, c2n=self.column2,
                                                                         en=new_freq, c1n=self.column1,
                                                                         word=word_name))
            return True
        return False

Solution

Obviously, you already know about SQL injection. So let's forget about that. Ideally, please do defend against it, it's easy enough that it's not worth doing it any other way. Code that is not public facing now, might be next week.

Some other things:

You use fetchall instead of fetchone. fetchall has to load all rows into memory (I assume), which is pretty expensive. Don't use it.

Also, you use SELECT *, which needs to load all columns (of the relevant rows) into memory. Again, expensive.

def check_word(self, word_name):
    self.c.execute('SELECT {c2n} FROM {tn} WHERE {c1n} = """{wn}"""'.format(tn=self.table1, c2n=self.column2, c1n=self.column1, word=word_name))
    data = self.c.fetchone()
    if data is not None:
        new_freq = data[0] + 1
        self.c.execute(
            'UPDATE {tn} SET {c2n}={en} WHERE {c1n}="""{word}"""'.format(tn=self.table1, c2n=self.column2,
                                                                     en=new_freq, c1n=self.column1,
                                                                     word=word_name))
        return True
    return False


Also, there is a bit of a trouble with race-conditions here, but that's probably easily solvable using transactions and locking. Or, better, use 1 query (which is also faster!)

def check_word(self, word_name):
    query = 'UPDATE {table_name} SET {counter} = {counter} + 1 WHERE {word_column} = ?'.format(
        table_name=self.table1,
        counter=self.column2,
        word_column=self.column1,
    )
    self.c.execute(query, word_name)
    return self.c.rowcount > 0


(Here, I use the rowcount attribute of a cursor which returns the number of rows changed in the last query: https://docs.python.org/2/library/sqlite3.html#sqlite3.Cursor.rowcount).

I tried to use more sensible names for the columns, and also used string-interpolation for the values that are hopefully not under user control (and can't be done using parametrized queries anyhow), while using paramatrisation for the word_name which is more likely to change.

Code Snippets

def check_word(self, word_name):
    self.c.execute('SELECT {c2n} FROM {tn} WHERE {c1n} = """{wn}"""'.format(tn=self.table1, c2n=self.column2, c1n=self.column1, word=word_name))
    data = self.c.fetchone()
    if data is not None:
        new_freq = data[0] + 1
        self.c.execute(
            'UPDATE {tn} SET {c2n}={en} WHERE {c1n}="""{word}"""'.format(tn=self.table1, c2n=self.column2,
                                                                     en=new_freq, c1n=self.column1,
                                                                     word=word_name))
        return True
    return False
def check_word(self, word_name):
    query = 'UPDATE {table_name} SET {counter} = {counter} + 1 WHERE {word_column} = ?'.format(
        table_name=self.table1,
        counter=self.column2,
        word_column=self.column1,
    )
    self.c.execute(query, word_name)
    return self.c.rowcount > 0

Context

StackExchange Code Review Q#124191, answer score: 7

Revisions (0)

No revisions yet.