7
\$\begingroup\$

I'm writing a small Python script which needs to fetch live data from a database (I'm using SQLite 3 for now). I may reuse this function for a lot of scripts. Can this code be made better?

def fetch_database(database, table, filter_dict, case=None):
    """
    Takes filter parameter as a dict
    dict = {'column_name':['filter', 'filter2'], 'column_name2'=[], ..}
    case = wrap around the dictionary key(UPPER, LOWER, etc..), None
    database (with pathname ex: database.db
    table
    column = dict.keys()
    """

    conn = sqlite3.connect(database)
    c = conn.cursor()
    filter_dict = dict_format(filter_dict) # A separate function deletes empty keys from dictionary
    keys_list = filter_dict.keys()
    statement = 'SELECT * FROM ' + table
    if len(keys_list) > 0:
        statement += ' WHERE '
        for keys in keys_list:
            if case == None:
                key = keys
            else:
                key = case + '(' + keys + ')'
            temp_data = filter_dict[keys]
            temp_size = len(temp_data)
            if keys_list.index(keys) != 0:
                statement += ' AND '
            if temp_size > 0:
                for data in temp_data:
                    if temp_data.index(data) == 0:
                        statement += key + '="' + data + '"'
                    else:
                        statement += ' OR ' + key + '="' + data + '"'
    dataset = c.execute(statement)
    dataset = dataset.fetchall()
    return dataset
\$\endgroup\$

2 Answers 2

8
\$\begingroup\$

When it comes to SQLite, I like to first wrap everything related to database into its own class:

class SqliteDB:
    def __init__(self):
        self.parent_directory = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
        self.db_path = os.path.join(self.parent_directory, 'db.sqlite3')
        self.db_connection = sqlite3.connect(self.db_path)
        self.db_cursor = self.db_connection.cursor()

    def execute(self, query, args=''):
        return self.db_cursor.execute(query, args)

    def commit(self):
        self.db_connection.commit()

    def close(self):
        self.db_connection.close()

That way, you can easily use the class into whatever project you need. Now, given the above, you can modify your function to take advantage of the new class:

def fetch_database(database, table, filter_dict, case=None):
    filter_dict = dict_format(filter_dict)
    keys_list = filter_dict.keys()
    statement = 'SELECT * FROM {}'.format(table)
    if len(keys_list) > 0:
        statement += ' WHERE '

        for keys in keys_list:
            if case is None:
                key = keys
            else:
                key = '{}({})'.format(case, keys)

            temp_data = filter_dict[keys]
            temp_size = len(temp_data)

            if keys_list.index(keys) != 0:
                statement += ' AND '

            if temp_size > 0:
                for data in temp_data:
                    if temp_data.index(data) == 0:
                        statement += '{}="{}"'.format(key, data)
                    else:
                        statement += ' OR {}="{}"'.format(key, data)

    return database.execute(statement).fetchall()

Then you can use it like this:

if __name__ == '__main__':
    database = SqliteDB()
    fetch_database(database, 'some_table', {'some': 'dict'}, case=None)

Other changes that I did to what you had:

  • I've used the .format() method instead of the deprecated + method
  • I've modified if case == None to if case is None.
  • I added if __name__ == '__main__' guard
\$\endgroup\$
3
  • \$\begingroup\$ seems like you should have mentioned how this is prone to sql injection attacks \$\endgroup\$
    – james
    Apr 27, 2017 at 15:30
  • \$\begingroup\$ (Its it is - first sentence.) \$\endgroup\$ Apr 27, 2017 at 18:26
  • 1
    \$\begingroup\$ @PeterMortensen thanks. I know the difference. It was just a typo \$\endgroup\$ Apr 27, 2017 at 19:46
11
\$\begingroup\$

I'm surprised nobody mentioned the code being vulnerable to SQL injection attacks:

Her daughter is named Help I'm trapped in a driver's licence factory.

(XKCD source)

Using string formatting and string concatenation to construct SQL queries is, first of all, unsafe and also, fragile - you will have to handle the argument types, balancing and escaping quotes yourself manually.

Instead, you should parameterize your queries - this way, you are not only protected from SQL injections, but also let the database driver worry about the python-to-database type conversions, sample:

query = """
    SELECT column1 
    FROM table
    WHERE column2 = ?"""
cursor.execute(query, (data, ))

Note how parameters are passed in a separate argument to execute().

On the other hand, it feels like you may be reinventing the wheel in a certain way - please check if switching to an ORM like SQLAlchemy or peewee would help to avoid re-writing boilerplate code.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ I wanted to say this, because I remembered I've even given an answer to a related question here but I don't know why I didn't do it ^_^. Nicely spot! \$\endgroup\$ Apr 27, 2017 at 9:09
  • 1
    \$\begingroup\$ Thanks for mentioning SQLAlchemy, I will check that. The user input is spell corrected (using personnel dictionary), classified & hence a dictionary format for filtering. It can't contain anything that is not listed. \$\endgroup\$ Apr 27, 2017 at 9:10
  • 2
    \$\begingroup\$ @MrGrj but you've still provided a very nice SQLite db Python wrapper! I've decided to add Bobby Tables's story here too :) Thanks. \$\endgroup\$
    – alecxe
    Apr 27, 2017 at 9:45

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.