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";-)
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
$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
Jared Williams wrote:
$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.
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
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
Am 31.03.2005 um 17:28 schrieb Marcus Boerger:
Hello Thies,
Friday, March 25, 2005, 1:55:30 PM, you 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!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.
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
Yep, will do.
so - again - lets make the colon optional - everywhere we allow
binding in PDO!
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 ..
-
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 -
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. -
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. -
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
Lukas Smith wrote:
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 ..
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
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.
Wez Furlong wrote:
How about sitting down together at php|tropics
Sure.
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.
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
I also noticed that bindParam() and bindColumn() are 1-indexed, which
will raise inconviniences with people using arrays for example to stored
the numeric indexesPassing 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
Hello Thies,
Friday, April 1, 2005, 3:13:10 AM, you wrote:
Am 31.03.2005 um 17:28 schrieb Marcus Boerger:
Hello Thies,
Friday, March 25, 2005, 1:55:30 PM, you 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!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.
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