From: Tim Chase on
Victor Subervi wrote:
> Hi;
> The following code works fine. I would like you to suggest something more
> simple and elegant:
>
> sql = 'select p.ID from %sPackages p join %sCategoriesPackages c where
> c.CategoryID=%s;' % (store, store, categoryID)
> cursor.execute(sql)
> tmp = [itm[0] for itm in cursor]
> packageIDs = []
> for t in tmp:
> if t not in packageIDs:
> packageIDs.append(t)

You mean like

sql = "select distinct p.ID from ..." % (...)
# ^^^^^^^^
cursor.execute(sql)
package_ids = [row[0] for row in cursor.fetchall()]

It would also help if you didn't pass the categoryID as a
string-formatted value, but as a proper parameter, something like

sql = "... where c.categoryid=?" % (store, store)
cursor.execute(sql, (category_id,))

This helps prevent SQL-injection attacks (assuming you have full
control over the value of "store"...otherwise, as you've been
advised, if the remote user has control over the value in
"store", you're asking to be exploited). You'd have to check the
place-holder character for your particular back-end:

>>> import <your database engine> as db
>>> print db.paramstyle

should tell you whether to use "?", "%s", or some other notation.



From: Steve Holden on
Victor Subervi wrote:
> On Sat, Jan 9, 2010 at 8:39 AM, Tim Chase <python.list(a)tim.thechases.com
> <mailto:python.list(a)tim.thechases.com>> wrote:
>
> Victor Subervi wrote:
>
> Hi;
> The following code works fine. I would like you to suggest
> something more
> simple and elegant:
>
> sql = 'select p.ID from %sPackages p join
> %sCategoriesPackages c where
> c.CategoryID=%s;' % (store, store, categoryID)
> cursor.execute(sql)
> tmp = [itm[0] for itm in cursor]
> packageIDs = []
> for t in tmp:
> if t not in packageIDs:
> packageIDs.append(t)
>
>
> You mean like
>
> sql = "select distinct p.ID from ..." % (...)
>
>
> Oh, that's good!
>
>
> # ^^^^^^^^
> cursor.execute(sql)
> package_ids = [row[0] for row in cursor.fetchall()]
>
> It would also help if you didn't pass the categoryID as a
> string-formatted value, but as a proper parameter, something like
>
> sql = "... where c.categoryid=?" % (store, store)
> cursor.execute(sql, (category_id,))
>
>
> I now have the following:
>
> sql = 'select distinct p.ID from %sPackages p join
> %sCategoriesPackages c where c.CategoryID=?;' % (store, store)
> cursor.execute(sql, (categoryID,))
> packageIDs = [itm[0] for itm in cursor]
>
> It threw this error:
>
> /var/www/html/angrynates.com/christians/cart/display.py
> <http://angrynates.com/christians/cart/display.py>
> 141 print '</td></tr></table>\n'
> 142 cursor.close()
> 143 bottom()
> 144
> 145 display()
> display = <function display>
> /var/www/html/angrynates.com/christians/cart/display.py
> <http://angrynates.com/christians/cart/display.py> in display()
> 109 categoryID = cursor.fetchone()[0]
> 110 sql = 'select distinct p.ID from %sPackages p join
> %sCategoriesPackages c where c.CategoryID=?;' % (store, store)
> 111 cursor.execute(sql, (categoryID,))
> 112 packageIDs = [itm[0] for itm in cursor]
> 113 for pid in packageIDs:
> global cursor = <MySQLdb.cursors.Cursor object>, cursor.execute = <bound
> method Cursor.execute of <MySQLdb.cursors.Cursor object>>, sql = 'select
> distinct p.ID from productsPackages p join productsCategoriesPackages c
> where c.CategoryID=?;', categoryID = 1L
> /usr/lib64/python2.4/site-packages/MySQLdb/cursors.py in
> execute(self=<MySQLdb.cursors.Cursor object>, query='select distinct
> p.ID from productsPackages p join productsCategoriesPackages c where
> c.CategoryID=?;', args=(1L,))
> 146 query = query.encode(charset)
> 147 if args is not None:
> 148 query = query % db.literal(args)
> 149 try:
> 150 r = self._query(query)
> query = 'select distinct p.ID from productsPackages p join
> productsCategoriesPackages c where c.CategoryID=?;', db = <weakproxy at
> 0x2b79db9dc470 to Connection>, db.literal = <bound method
> Connection.literal of <_mysql.connection open to 'localhost' at
> 142be8b0>>, args = (1L,)
>
> TypeError: not all arguments converted during string formatting
> args = ('not all arguments converted during string formatting',)
>
>
> This helps prevent SQL-injection attacks (assuming you have full
> control over the value of "store"...otherwise, as you've been
> advised, if the remote user has control over the value in "store",
> you're asking to be exploited).
>
>
> They have control over it. I pass it in the url. Please advise.
>
>
> You'd have to check the place-holder character for your particular
> back-end:
>
> >>> import <your database engine> as db
> >>> print db.paramstyle
>
> Printed "format". What's that mean? I use MySQLdb
> TIA,
> beno
>
Given that you actually started this thread by asking a good question
that showed you had done some independent work, I'll bite.

The problem is something that was discussed in one of your other
numerous threads by John Machin and me. The issue is the
parameterization of (i.e. sticking variable bits into) SQL queries.

When you write

curs.execute("some sql query with %s and %s in it", (data1, data2))

the second argument to execute is supposed to contain data values. This
allows the SQL engine to do the preparatory work for a query once, and
then use the same "prepared query" then next time it's executed. The
preparation involves scanning the SQL query to make sure the syntax is
correct, validating the table and column names, and developing a "query
execution plan" that is a sequence of internal operations the database
performs to get you the answer you want. (SQL, unlike Python, is a
"declarative" language - rather than telling it what to do you describe
the results you want to see and the engine works out how to provide it).

Of course, if different tables are used for different queries then there
is no hope that the same execution plan can be used for them. For this
reason most database processors (and this certainly includes the one you
are using) don't allow you to parameterize anything in SQL statement
other than data values. So

curs.execute("select field1 from tableA where id=%s", (3, ))

is OK, but

curs.execute("select field1 from tableA where %s=3", ("id", ))

is definitely not. And

curs.execute("select field1 from %stable where id=3", (storename, ))

is right out.

This goes back to the issue of your database design. You have chosen to
represent each store with its own set of tables, where an experienced
database designer would instead have used just one set of tables, each
table having a "store" column that would allow you to use parameterized
queries to select the relevant data. There are many other sound reasons
for making such a design choice, which primarily boil down to
operational and management efficiency and simplicity.

But we are now in the realm of theory as far as you are concerned, since
you have already stated several times that you aren't interested in
correcting your design until after you have got the current mess into
production. So good luck with that.

regards
Steve

--
Steve Holden +1 571 484 6266 +1 800 494 3119
PyCon is coming! Atlanta, Feb 2010 http://us.pycon.org/
Holden Web LLC http://www.holdenweb.com/
UPCOMING EVENTS: http://holdenweb.eventbrite.com/

From: Gabriel Genellina on
En Sat, 09 Jan 2010 11:01:25 -0300, Victor Subervi
<victorsubervi(a)gmail.com> escribi�:
> On Sat, Jan 9, 2010 at 8:39 AM, Tim Chase
> <python.list(a)tim.thechases.com>wrote:
>
>> It would also help if you didn't pass the categoryID as a
>> string-formatted
>> value, but as a proper parameter, something like
>>
>> sql = "... where c.categoryid=?" % (store, store)
>> cursor.execute(sql, (category_id,))
>>
>
> I now have the following:
>
> sql = 'select distinct p.ID from %sPackages p join
> %sCategoriesPackages c where c.CategoryID=?;' % (store, store)
> cursor.execute(sql, (categoryID,))
> packageIDs = [itm[0] for itm in cursor]
>
> It threw this error:
>
> TypeError: not all arguments converted during string formatting
> args = ('not all arguments converted during string formatting',)
>
>> You'd have to check the place-holder character for your particular
>> back-end:
>>
>> >>> import <your database engine> as db
>> >>> print db.paramstyle
>>
>> Printed "format". What's that mean? I use MySQLdb

That means, MySQLdb uses %s as a placeholder for parameter substitution --
same as Python when doing string interpolation. Unfortunately this will
confuse things. In your code above, the ? near the end should become %s --
but you don't want THAT %s to be interpreted by Python at that time,
instead it must remain as a literal %s until the cursor.execute line. You
have to escape the % by doubling it: %%s

sql = 'select distinct p.ID from %sPackages p join
%sCategoriesPackages c where c.CategoryID=%%s;' % (store, store)
cursor.execute(sql, (categoryID,))
packageIDs = [itm[0] for itm in cursor]

--
Gabriel Genellina