Passing the correct SQL Statement - what is the correct way?

Question

My code is following

selection_option_heading_1 = request.form.get("selection_option_heading").lower()
search_string_1 = request.form.get("search_string").lower()
search_string_1 = ("'%"+search_string_1+"%'")
check_books_in_db = db2.execute(f"SELECT isbn, title, author, year FROM books WHERE {selection_option_heading_1} ILIKE {search_string_1} LIMIT 50").fetchall()
return (check_books_in_db)

It is working.

But I know there should be a proper way to do this. Any help?

Proper way would be to use an ORM so that your code will be independent of your backend database. https://docs.sqlalchemy.org/en/latest/orm/ Please add more details like what connector you use etc. — Nihal Sangeeth, Mar 12 '19 at 07:45
The tag `psql` makes this look like a PostgreSQL database, but psql is the interactive shell, not a Python module. Which driver do you use? — shmee, Mar 12 '19 at 07:56
@DipeshSukhani A bit, yes, Which dialect? The default `postgresql` (which is in fact `psycopg2`)? I.e. does your engine creation look like this: `engine = create_engine('postgresql://)`? — shmee, Mar 12 '19 at 08:59
On an aside: If you are using an ORM, shouldn't your query rather look like sth. along the lines of `session.query(Book).filter(getattr(User, 'selection_option_heading_1').ilike("'%"+search_string_1+"%'").limit(50)`? I mean, what's the point of using an ORM if you fire raw SQL against it? — shmee, Mar 12 '19 at 09:22
Yes shmee :) not yet reached the stage of ORM in my learning — AmateurDev, Mar 12 '19 at 09:35
@DipeshSukhani Well, that does not really help :) Leaving the whole ORM aspect aside, the answer from @shikaing below looks quite OK. When it comes to queries in SQLAlchemy, this question unfortunately lacks information, e.g. the mapping belonging to your `books` table, in case you have one. — shmee, Mar 12 '19 at 10:08

shikai ng · Answer 1 · 2019-03-12T08:29:00.170

Generally you do not inject user input directly into SQL statements like that, since it would make your code vulnerable to sql injection attacks. If you are using python to perform SQL query, the standard mysql.connector library provides relatively safer way of performing queries based on user input.

Since selection_option_heading_1 is a column name, it would be best to check if such a column exists before querying it.

An example would be :

SQL_col_query = "SELECT * FROM information_schema.columns
                 WHERE table_schema = 'your_schema'
                 AND table_name   = books"
cur.execute(SQL_col_query)
column_names = [row[0] for row in cur.fetchall()]
if ( selection_option_heading_1 in column_names) :
    SQL_query = f"SELECT isbn, title, author, year FROM books WHERE {selection_option_heading_1} ILIKE %S LIMIT 50"
    data = (selection_option_heading_1 , search_string_1 )
    cur.execute(SQL_query , data)
    ...

This is definitely a safer alternative to what you are doing, but I wouldnt be clear if it is the best way to perform such queries.

I would give you +1 for warning about SQL injection attacks right away, but: 1) the first "parameter" you pass (`WHERE %s ILIKE`) is actually a column name; you can't provide these like parameters for substitution in the `execute` call and 2) this doesn't look like a MySQL DB. The tag `psql` and the use of `ILIKE` (which MySQL doesn't support) looks rather like PostgreSQL. :) — shmee, Mar 12 '19 at 08:06

Passing the correct SQL Statement - what is the correct way?

1 Answers1