The case against Prepared Statements

I see SQL code here that does not use Prepared Statements, and it concerns me. If you are querying data, there are very few times where it’s legitimate write straight SQL, so I wanted to list the arguments against PS’s here.

  • I don’t know how. Using a Prepared Statement is actually simple and often easer to write. The point is to separate your data from the instructions to the SQL engine so it’s not possible to inadvertently do something bad. (This is known as “SQL injection”.) It’s worth the time to learn.
  • I clean the data myself. You shouldn’t. The SQL engine already knows how to do this, provides a mechanism for it, and will always do it right, now and in the future.
  • I control the inputs. This one is legitimate as long as we aren’t talking about text. If you are querying numbers, booleans, or something else that isn’t quoted and does not come from the user, go ahead and join the statement yourself. Otherwise, let the engine do it even when you’re providing the text.

Finally, consider this code:

dim sql as string = _
    "SELECT * FROM my_table WHERE " + _
    col1 = '" + fld1.Clean + "' OR " + _
    (col2 LIKE '%" + fld2.Clean + "%' AND col3 = '" + fld3.Clean + "') OR " + _
    "col4 = " + str( int1 )

If this were written as a Prepared Statement, it would be:

dim sql as string = _
    "SELECT * FROM my_table WHERE " + _
    "col1 = ? OR " + _
    "(col2 LIKE ? AND col3  = ?) OR " + _
    "col4 = ?"

If nothing else, isn’t that easier to read?

Use Prepared Statements.

Just a hint: You can combine PreparedStatement.Bind and PreparedStatement.BindType in a onlineliner:

PreparedStatement.Bind(0, theData, PreparedStatement.DataType)

To adapt to the example from Kem:

Dim ps As PreparedStatement
Dim rs As RecordSet

<Here is then Kem’s Code>

ps = Database.Prepare(sql)

ps.Bind(0, theDatathatreplacestheQuestionmark, PreparedStatement.DataType)
ps.Bind(1, theDatathatreplacestheQuestionmark, PreparedStatement.DataType)
ps.Bind(2, theDatathatreplacestheQuestionmark, PreparedStatement.DataType)
ps.Bind(3, theDatathatreplacestheQuestionmark, PreparedStatement.DataType)

rs = ps.SQLExecute

what I have against preparedstatements, is that the code will no longer be cross database engine
a sql query may run under sqlite and postgres with no change
but the corresponding preparedstatements have to be written differently on the two engines
that is annoying in multi databases environment.

I use prepared statements whenever I am doing a query or operation based on user input. If a user enters a ’ character, and you are not using prepared statements it breaks things.

However, I use a ton of SQLSelect and other statements that are hard coded based on user actions. In that case, I see no reason to use a prepared statement because there’s no way the user can inject anything into the query in this case. In fact, they don’t even know it’s going on.

[quote=381238:@Jean-Yves Pochez]what I have against preparedstatements, is that the code will no longer be cross database engine
a sql query may run under sqlite and postgres with no change
but the corresponding preparedstatements have to be written differently on the two engines
that is annoying in multi databases environment.[/quote]

That is a concern, although, practically speaking, how often does the average developer change database engines? And if you really need to, you can write a parser that does the work for you in the background.

Also, you can always use a SQL builder to bypass the issue entirely. I happen to have one available… :slight_smile:

https://github.com/ktekinay/XOJO-SQLBuilder

As Kem said, whenever you work with Text/Strings from User Input or even if you pull it from any other 3rd Party Source like the Database, a File, …, using Prepared Statements is highly recommended. Not doing this, will open possibilitys to inject Code where it’s not wanted. :slight_smile:

That was my last bullet point, and I agree, you can skip the PS if you control the inputs unless you want to guard against your own error where text is involved.

I agree wholeheartedly with the use of prepared statements. However, there are some major issues with the MSSQL Xojo Plugin that make using prepared statements nearly impossible, for at least MS SQL. I hope at some point this gets a review because that plugin does need some work. Here’s a sampling of links that describe the issues with native plugin for MSSQL.

link text

link text

link text

link text

I’m only mentioning this because although these issues are not a “case against prepared statements” they do present a technical challenge in implementing the best-practice of actually using them.

Woah! thanks for that Sascha!

If you use MBS SQL Plugin for databases, you can use named parameters:

compare this by index:

[code]dim r as PreparedSQLStatement = db.Prepare(“Insert into test_tbl(fid, fvarchar20) values(:1, :2)”)

r.BindType(0, SQLPreparedStatementMBS.kTypeLong)
r.BindType(1, SQLPreparedStatementMBS.kTypeString)

r.SQLExecute 12345, “Hello World by index”[/code]

With this by name:

[code]dim sql as string = “Insert into test_tbl(fid, fvarchar20) values(:fid, :fvarchar20)”
dim v as Variant = db.Prepare(sql)
dim p as SQLPreparedStatementMBS = v

p.BindType(“fid”, SQLPreparedStatementMBS.kTypeLong)
p.BindType(“fvarchar20”, SQLPreparedStatementMBS.kTypeString)
p.Bind(“fid”, 2345)
p.Bind(“fvarchar20”, “Hello World by name”)

p.SQLExecute[/code]

I’d love to see that in the Xojo API, too.
As well as a couple of other improvements.

If those PreparedStatements just wouldn’t crash randomly for SQLite… it works fine when the PreparedStatement is a local variable. Make it a property of the class and get random crashes.

Why do you do that? (just curious)
Would a workaround be having a method that generates a fresh PS each time you need one?
As far as I know they’re not hard for the executable to make, so I don’t think it would be a performance impact.

Also, Postgres does not need BindType. It figures that out itself :wink:

@Tim Parnell: well, I thought that there would be a performance improvement when I reuse the statement. But you are correct, there wasn’t one.

The PreparedStatement isn’t that complex:

 dim thePreparedStatement as SQLitePreparedStatement = SQLiteIndexDB.Prepare("INSERT INTO bodyindex(docid, messagebody) VALUES(?, ?)")

The table is a dataless one and I only write index data to it. I never got the crash myself but I have at least half a dozen crash reports:

Thread 12 Crashed:
0 com.xojo.XojoFramework 0x024fbe5f 0x22fa000 + 2104927
1 com.xojo.XojoFramework 0x024fbcbf 0x22fa000 + 2104511
2 com.xojo.XojoFramework 0x024c761d 0x22fa000 + 1889821
3 SQLiteDatabase.dylib 0x08677409 0x8676000 + 5129
4 SQLiteDatabase.dylib 0x08681132 0x8676000 + 45362
5 SQLiteDatabase.dylib 0x086814b7 0x8676000 + 46263
6 SQLiteDatabase.dylib 0x08680940 0x8676000 + 43328
7 com.mothsoftware.mailarchiverx 0x00143bf1 SQLitePreparedStatement.*SQLExecute%%oA1v + 72
8 com.mothsoftware.mailarchiverx 0x01592d46 SQLiteIndex.Write%%oi4s + 784
9 com.mothsoftware.mailarchiverx 0x00ec0793 MailParser.parse%i4%o + 10266
10 com.mothsoftware.mailarchiverx 0x00e5e751 ArchiveThread.Archive%%o +

I wrote SQLdeLite to make using prepared statements a no brainer: https://github.com/1701software/sqldelite

You can use it as needed, requires little to no re-training and does the right thing with prepared statements behind the scenes.

[quote=381304:@Tim Parnell]Why do you do that? (just curious)
Would a workaround be having a method that generates a fresh PS each time you need one?
As far as I know they’re not hard for the executable to make, so I don’t think it would be a performance impact.[/quote]

I tend to reuse properties too and ran into what Beatrix said. Now I just make new local PS every time. Annoying that PS can’t be reused… But it’s an easy fix.

I use prepared statements mostly for text so I have and extension method called BindAll.

[code]Sub BindAll(extends SP as SQLitePreparedStatement, Num as Integer, BindType as Integer = SQLitePreparedStatement.SQLITE_TEXT)
if num < 1 then return

for i as Integer = 0 to num -1
sp.BindType(i,BindType)
next
End Sub
[/code]

Then use it like this:

dim ps as SQLitePreparedStatement
ps = db.Prepare("INSERT INTO Temp(Category, Item, Description,Qty) VALUES (?,?,?,?)")
ps.BindAll 4
ps.SQLExecute(Category, Item, Description, Qty)

Here’s another tip, SQLite supports numbered parameters. So you can do

SELECT first_name, last_name WHERE first_name LIKE ?1 OR last_name_like ?1;

And provide only one parameter. It means you can also put them out of order, so you could put all your values into an array and do an update or insert.

[code]Dim Values() As Variant
Values(0) = RowID
Values(1) = Label

Dim UpdateStatement As SQLitePreparedStatement = Database.Prepare(“UPDATE objects SET label = ?2 WHERE rowid = ?1;”)
Dim InsertStatement As SQLitePreparedStatement = Database.Prepare(“INSERT INTO objects (rowid, label) VALUES (?1, ?2);”)
// Bind

Dim Results As RecordSet = Database.SQLSelect(“SELECT * FROM objects WHERE condition = true;”)
If Results.RecordCount = 1 Then
UpdateStatement.SQLExecute(Values)
Else
InsertStatement.SQLExecute(Values)
End If[/code]

This example is of dubious utility though, it’s more useful if you’re managing a bunch of records so you only prepare and bind once. It’s also much faster than an INSERT OR REPLACE statement, surprisingly.

Lastly, the in most of my projects I like to create a database subclass that improves SQLSelect and SQLExecute by allowing you to pass values directly to the method. You can see a real-world example of this in https://github.com/thommcgrath/Beacon/blob/master/Project/Data%20Model/LocalData.xojo_code around line 880 (code may change so line numbers won’t stay precise)

Then prepared statements become as simple as

Dim Results As RecordSet = Database.SQLSelect("SELECT * FROM objects WHERE label = ?1;", "Foo")

Which, in my opinion, is prepared statement bliss. I really wish this is how they worked out of the box, but I understand that it’s one thing to setup for my own project and something else entirely to setup a perfect version for everybody.

[quote=381238:@Jean-Yves Pochez]what I have against preparedstatements, is that the code will no longer be cross database engine
a sql query may run under sqlite and postgres with no change
but the corresponding preparedstatements have to be written differently on the two engines[/quote]
I’ve written myself a class which encapsulates the complete preparedstatements technique for me, and a lot more, and the SQL that goes in is equal for MySQL, Postgress, SQLserver and SQLite. The code within the class does the abstraction.
The funny thing is that I allays use the technique recommended by @Kem Tekinay , without even realizing I do and with the ease of just a few lines of simple straight forward code. Cannot live without anymore. :slight_smile:

One of the goals in Xanadu, is avoid writing table specific code. When updating table columns, Xanadu calls “DESCRIBE Contacts ;” which returns:

Xanadu then parses the Type column to get the base Type ‘decimal’, the Length of the Integer portion ‘20’, and the Length of the Decimal portion ‘2’. With that column Type info, Xanadu queries the record using/binding only the table ID and then uses Record.Edit, a method that uses the DESCRIBE Type to set Record.Fields, and then Record.Update.

The same concept could be used to create a method like @Neil Burkholder suggested by Binding columns on the fly based on the column Type.

As @Kem Tekinay stated, binding is critical.