# pdo: binding variables supplied to execute() is NotVeryUseful(tm)...

- Thread: https://externals.io/message/15594
- Original archive: https://news-web.php.net/php.internals/15594
- Started by Thies C. Arntzen on 2005-03-25T11:55:30+00:00
- Messages: 13
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #15594 — Thies C. Arntzen — 2005-03-25T11:55:30+00:00

URL: https://externals.io/message/15594#15594

hi,

pdo hat it's own query-parser, named variables are prefixed with a 
colon... so far - so nice...

i have a function called insert which is called like this:

$db->insert('some_table', array('name' => $name, 'age' => $age));

it's implemented like this:

function insert($table, $values) {
     $column_names = array_keys($values);
     $col_list = implode(', ', $column_names);
     $bind_list = implode(', :', $column_names);

     $sql = "insert into $table ($col_list) values ($bind_list)";
	
     $stmt = $this->prepareStatement($sql);

     // works:
     // this is how it has to be done..
     //
     foreach ($values as $col => $val) {
         $stmt->bindParam(":$col", $values[ $col ]);
     }
     $stmt->execute();

     // doesn't work
     // and this is how i would like it instead
     //
     # $stmt->execute($values);
}


and this is 'cause pdo _expects_ the user to prefix the bound variables 
with a colon. grr...

ppls, lets change it before it's too late. this "tiny bit" makes binding 
harder than it should be, and we all know and understand that all user 
of php should use bound variables all the time!

suggestion: auto-trim a leading colon from the internal binding tables. 
that way "oldish" code would not break...


re, thies

PS: looking into that right now - hopefully "patch follows";-)

---

## #15595 — Lukas Smith — 2005-03-25T11:57:12+00:00

URL: https://externals.io/message/15594#15595 · In reply to: #15594

Thies C. Arntzen wrote:

> and this is 'cause pdo _expects_ the user to prefix the bound variables 
> with a colon. grr...
> 
> ppls, lets change it before it's too late. this "tiny bit" makes binding 
> harder than it should be, and we all know and understand that all user 
> of php should use bound variables all the time!

you are not alone:
http://oss.backendmedia.com/index.php?area=PDO&page=bindParam

regards,
Lukas

---

## #15598 — Jared Williams — 2005-03-25T13:36:52+00:00

URL: https://externals.io/message/15594#15598 · In reply to: #15594

>      $sql = "insert into $table ($col_list) values ($bind_list)";

Can I just point out that you've just negated the whole reason for having parameters in the first place, imo.

$table is just as vulnerable to an SQL injection attack, as any of the parameters where before we had parameter binding.

Jared

---

## #15599 — Lukas Smith — 2005-03-25T13:41:59+00:00

URL: https://externals.io/message/15594#15599 · In reply to: #15598

Jared Williams wrote:
> > […]
> 
> 
> Can I just point out that you've just negated the whole reason for having parameters in the first place, imo.

uhm the point of prepared queries is not to eliminate sql injection.
thats just an added benefit

> $table is just as vulnerable to an SQL injection attack, as any of the parameters where before we had parameter binding.

you are assuming that $table has not bee sanitized, which seems quite 
unlikely to me that its even going to be a variable controled by user 
input in the first place.

regards,
Lukas

---

## #15700 — Marcus Boerger — 2005-03-31T15:28:38+00:00

URL: https://externals.io/message/15594#15700 · In reply to: #15594

Hello Thies,

Friday, March 25, 2005, 1:55:30 PM, you wrote:

> hi,

> pdo hat it's own query-parser, named variables are prefixed with a 
> colon... so far - so nice...

> i have a function called insert which is called like this:

> $db->insert('some_table', array('name' => $name, 'age' => $age));

> it's implemented like this:

> function insert($table, $values) {
>      $column_names = array_keys($values);
>      $col_list = implode(', ', $column_names);
>      $bind_list = implode(', :', $column_names);

>      $sql = "insert into $table ($col_list) values ($bind_list)";
	
>      $stmt = $this->prepareStatement($sql);

>      // works:
>      // this is how it has to be done..
>      //
>      foreach ($values as $col => $val) {
>          $stmt->bindParam(":$col", $values[ $col ]);
>      }
>      $stmt->execute();

>      // doesn't work
>      // and this is how i would like it instead
>      //
>      # $stmt->execute($values);
> }


> and this is 'cause pdo _expects_ the user to prefix the bound variables 
> with a colon. grr...

> ppls, lets change it before it's too late. this "tiny bit" makes binding 
> harder than it should be, and we all know and understand that all user 
> of php should use bound variables all the time!

> suggestion: auto-trim a leading colon from the internal binding tables. 
> that way "oldish" code would not break...


> re, thies

> PS: looking into that right now - hopefully "patch follows";-)


We could easily add this behavior and i think it makes somewhat sense.
It just looks a bit like __wakepup/__sleep. The only 'but' is that i
suggest the behavior has its own method like 'bindParamArray'. If you
cannot work out a patch feel free to contact me. Maybe i'll find some
time during the conf.

-- 
Best regards,
 Marcus                            mailto:mail@marcus-boerger.de

---

## #15711 — Thies C.Arntzen — 2005-04-01T08:13:10+00:00

URL: https://externals.io/message/15594#15711 · In reply to: #15700

Am 31.03.2005 um 17:28 schrieb Marcus Boerger:

> Hello Thies,
>
> Friday, March 25, 2005, 1:55:30 PM, you wrote:
>
> > […]
>
> > […]
>
> > […]
>
>
> > […]
>
> > […]
>
>
> We could easily add this behavior and i think it makes somewhat sense.
> It just looks a bit like __wakepup/__sleep. The only 'but' is that i
> suggest the behavior has its own method like 'bindParamArray'. If you
> cannot work out a patch feel free to contact me. Maybe i'll find some
> time during the conf.

hey marcus!

i don't quite understand the relation to sleep/wakeup here (i'm not 
reading all the php lists, maybe you could point me to the right 
thread?)

adding a new 'bindParamArray' with different semantics doesn't really 
make things easier. and i think we should have once consistent 
parameter-bind interface:

- a method bindParam
- an optinal associative array passed to execute.

all those functions expect to bind a <parameter name> to a statement. 
this <parameter name> should work both with or without a leading colon.

by the way (just checked) - this is how the "native" oracle-oci binding 
functions work. so oracle decided to make the colon in front of the 
placeholder for bind optional. at the end of the day you can only bind 
to a placeholder defined in your sql-statement. in the sql you have to 
"mark" the placeholder by prepending a colon - but for the bind you 
really don't need the colon. really!

so - again - lets make the colon *optional* - everywhere we allow 
binding in PDO!

re, thies

---

## #15712 — Wez Furlong — 2005-04-01T13:25:45+00:00

URL: https://externals.io/message/15594#15712 · In reply to: #15711

Yep, will do.


> so - again - lets make the colon *optional* - everywhere we allow
> binding in PDO!

---

## #15715 — Marcus Boerger — 2005-04-01T15:00:18+00:00

URL: https://externals.io/message/15594#15715 · In reply to: #15711

Hello Thies,

Friday, April 1, 2005, 3:13:10 AM, you wrote:


> Am 31.03.2005 um 17:28 schrieb Marcus Boerger:

> > […]

> hey marcus!

> i don't quite understand the relation to sleep/wakeup here (i'm not 
> reading all the php lists, maybe you could point me to the right 
> thread?)

> adding a new 'bindParamArray' with different semantics doesn't really 
> make things easier. and i think we should have once consistent 
> parameter-bind interface:

> - a method bindParam
> - an optinal associative array passed to execute.

> all those functions expect to bind a <parameter name> to a statement. 
> this <parameter name> should work both with or without a leading colon.

> by the way (just checked) - this is how the "native" oracle-oci binding 
> functions work. so oracle decided to make the colon in front of the 
> placeholder for bind optional. at the end of the day you can only bind 
> to a placeholder defined in your sql-statement. in the sql you have to 
> "mark" the placeholder by prepending a colon - but for the bind you 
> really don't need the colon. really!

> so - again - lets make the colon *optional* - everywhere we allow 
> binding in PDO!

> re, thies


hmmm,

this is just a copy'n'paste mistake somehow....

-- 
Best regards,
 Marcus                            mailto:mail@marcus-boerger.de

---

## #15719 — Lukas Smith — 2005-04-01T21:05:11+00:00

URL: https://externals.io/message/15594#15719 · In reply to: #15712

Wez Furlong wrote:
> Yep, will do.

I dont want to pretend that this change should open the flood gates for 
radical changes. However there are still a few minor points beyond the 
bindParam() that may need minor adjustments. Some of them can be added 
later of course ..

1) DSN http://oss.backendmedia.com/index.php?area=PDO&page=DSN
It would be nice to be able as an array and for the string format it 
would be nice to also have the option of using the established PEAR DSN 
format

2) `exec()` http://oss.backendmedia.com/index.php?area=PDO&page=exec
It may be nice to have a simple one shot prepared query to be able to do 
a prepare/execute in one call.

3) setAttribute/getAttribute 
http://oss.backendmedia.com/index.php?area=PDO&page=setAttribute
Some of the "attributes" are actually more like options that can be 
defined by the user. Others are defined by the client and yet others are 
defined by the server and cannot be changed by the user. I think atleast 
the user definable options should be moved to a separate 
setOption/getOption set of methods.

4) minor API stuff in the fetch methods
I think it would be clearer to call the fetch() method fetchRow() 
because that is what the method does. I also dont think we should 
default to PDO_FETCH_BOTH. It may also be a nice touch towards 
PEAR::[M]DB[2] to rename the fetchSingle() method to fetchOne(). Its 
also more clearer since fetchSingle() might as well imply a single row. 
Fetching a single row should probably be handled by a fetchCol() method.

regards,
Lukas

---

## #16075 — Lukas Smith — 2005-04-25T13:41:48+00:00

URL: https://externals.io/message/15594#16075 · In reply to: #15719

Lukas Smith wrote:
> Wez Furlong wrote:
> 
> > […]
> 
> 
> I dont want to pretend that this change should open the flood gates for 
> radical changes. However there are still a few minor points beyond the 
> bindParam() that may need minor adjustments. Some of them can be added 
> later of course ..

I also noticed that bindParam() and bindColumn() are 1-indexed, which 
will raise inconviniences with people using arrays for example to stored 
the numeric indexes, similar in how having to add the ":" was a problem 
for people using arrays to store the string parameters.

Beyond that rowCount() is more traditionally affectedRows() etc which 
needlessly break with existing method names in both php core extensions 
and PEAR packages. I would really like to avoid adding even more names 
and instead sticking with picking names from this already fairly vast 
name pool. It might even be a good idea to pick from the names most 
commonly used. I have attached a reference to the document I once 
drafted for this exact purpose [1]. I dont know where the names in PDO 
originate from. They dont seem to match the current oci8 extension (I 
first assumed these originated from the fact that alot of the API 
matured during your work on the oracle driver), but this does not seem 
to be the case.

I know I am getting on your nerves Wez, but I really think that the API 
should at this stage still be open to optimizing the API. While its no 
longer beta I still think its a bit premature for comments like these:

"There are releases on pecl.php.net, and there are (a few) people
running these in production. In addition, it's been advertised as
working this way for over a year."

How about sitting down together at php|tropics (I didnt plan on going to 
a'dam .. dunno about you Wez .. but if necessary I will travel to a'dam 
for a day) and simply going through each of the methods one by one. 
Looking at what kind of methods names we currently are using for the 
same or similar things in any of the current projects on php.net and 
make any adjustments then?

[1] http://marc.theaimsgroup.com/?l=php-dev&m=106440782305693&w=2

regards,
Lukas

---

## #16077 — Wez Furlong — 2005-04-25T15:15:24+00:00

URL: https://externals.io/message/15594#16077 · In reply to: #16075

> I also noticed that bindParam() and bindColumn() are 1-indexed, which
> will raise inconviniences with people using arrays for example to stored
> the numeric indexes

Passing an array with numeric indices to execute() expects 0 based
indices, to cater for this case.  Manually calling a bind function
expects 1 based indices, as this is the convention for the majority of
database APIs.

> Beyond that rowCount() is more traditionally affectedRows() etc which
> needlessly break with existing method names in both php core extensions
> and PEAR packages.

Look, PDO is new.  It's not breaking anything.  Stop claiming that it
is breaking things; it doesn't touch your code; it doesn't change the
external API of your code.  It doesn't make MDB, PEAR DB or any other
code change its names.  It's just different.  Deliberately.

> How about sitting down together at php|tropics

Sure.

> (I didnt plan on going to
> a'dam .. dunno about you Wez .. but if necessary I will travel to a'dam
> for a day)

That's a hell of a commute for me.

> and simply going through each of the methods one by one.
> Looking at what kind of methods names we currently are using for the
> same or similar things in any of the current projects on php.net and
> make any adjustments then?

Maybe, but I don't see a compelling reason to change the names, or I
would have done it already.
 
--Wez.

---

## #16118 — Lukas Smith — 2005-04-27T16:02:09+00:00

URL: https://externals.io/message/15594#16118 · In reply to: #16077

Wez Furlong wrote:

> > […]
> 
> 
> Sure.

> > […]
> 
> 
> Maybe, but I don't see a compelling reason to change the names, or I
> would have done it already.

Ok great!

We will see how much interest there is from others. Alot of these things 
I have already run by you so if its just me and you there is little 
reason to bother :-)

regards,
Lukas

---

## #16119 — Derick Rethans — 2005-04-27T16:31:14+00:00

URL: https://externals.io/message/15594#16119 · In reply to: #16077

> 
> > […]
> 
> Passing an array with numeric indices to execute() expects 0 based
> indices, to cater for this case.  Manually calling a bind function
> expects 1 based indices, as this is the convention for the majority of
> database APIs.

But it's totally not what PHP does now. Everything is 0 based so why 
make it *harder* for users by requiring a 1-based index here?

regards,
Derick

-- 
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org
