Support required for named SQL parameters

Bug #943159 reported by Don Schoeman
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
PHPDevShell
Status tracked in Trunk-3.x
Trunk-3.x
In Progress
Low
Greg

Bug Description

The PHPDS_db and PHPDS_query classes needs to support named SQL parameters, this will help future compatibility with PDO Objects as well as allow for the formatting of SQL statements to be improved upon before we fully support PDO.

An example of currently used non-parameter based queries:
$sql = "INSERT INTO sometable (foo, bar, baz) VALUES (%d, '%s', '%s')"

To add data into the query you would then usually do something like:
$query = sprintf($sql, 1, 'text', 'some more text');

Although the above example is extremely simple, the ordering of the parameters is extremely important and in complex queries with many fields it is easy to misalign the format parameters with the fields given.

To solve this problem PDO supports named SQL parameters (as does most other non PHP mysql database API's):
Example:
 $sql = "INSERT INTO sometable (foo, bar, baz) VALUES (:foo, ':bar', ':baz')"

For now, a PHPDevShell a function could then take a named array and simply replace the named parameters with the actual values, for example:
$query = $this->fmtParamSQL($sql, array(':foo' => 1, ':bar' => 'text', ':baz' => 'some more text'));

The fmtParamSQL() function can work like this:

function fmtParamSQL($sql, $params = false) {
 $result = $sql;
 foreach($params as $name => $value) {
  $result = str_ireplace(':'.$name, $value, $result);
 }
 return $result;
}

I propose that we build an additional function into PHPDS_db called invokeNamedQuery() to differentiate between the standard invokeQuery() function and a function which supports named parameters. This is going to take quite a lot of work since all the supporting functions will also then have to support named parameters.

If we convert all queries to named queries eventually we will then be able to support PDO and NoSQL much easier in the future.

Revision history for this message
TitanKing (titan-phpdevshell) wrote :

As per our discussion, experiment with solution, we will then hold a global communication and dicscuss our direction.

Revision history for this message
Don Schoeman (don.sch) wrote :

The future discussion will basically be about implementing PDO fully. Since PHPDS_query class already implements named arguments in a Python based syntax $(name) it should actually not be too difficult to support SQL based named parameters as I have described above. Perhaps we could simply add a protected $parameter_type option which either specifies 'python' or 'sql' or something along those lines. That option then tells the PHPDS_query::build() function which format to use.

Revision history for this message
Greg (gregfr) wrote :

Maybe I don't understand, but PHPDS_query already support named parameters:

INSERT INTO sometable (foo, bar, baz) VALUES (%(foo)s, %(bar)s);

$this->db->invokeQueryWith('MY_Query', array('foo' => 'FOO', 'bar' => 'BAR));

in this example my query is auto protected.

Revision history for this message
Greg (gregfr) wrote :

Sorry I didn't see you last comment. Do you have any documentation on the syntax you want?

Changed in phpdevshell:
assignee: nobody → Greg (gregfr)
Revision history for this message
Don Schoeman (don.sch) wrote :

Hi Greg, take a look at the first answer at StackOverflow, the question is not the same but the answer is relevant:
http://stackoverflow.com/questions/60174/best-way-to-stop-sql-injection-in-php

Basically, the format is extremely simple, instead of using a query like this:
INSERT INTO sometable (foo, bar, baz) VALUES (%(foo)d, '%(bar)s', '%(baz)')

you should be able to use:
INSERT INTO sometable (foo, bar, baz) VALUES (:foo, ':bar', ':baz')

a neat feature of some frameworks is to detect whether the parameter values you are sending is a string or not so that you don't even need to add the quotes, e.g:
INSERT INTO sometable (foo, bar, baz) VALUES (:foo, :bar, :baz)

The standard to format these SQL statements into proper statements is to pass the formatting function a named array, e.g.:
$sql = "INSERT INTO sometable (foo, bar, baz) VALUES (:foo, :bar, :baz)";
formatSQL($sql, array(':foo' => 1, ':bar' => 'test'field a, ':baz' => 'test field b')

Notice that the ':' character is added to the front of the parameter name, the formatSQL() should not magically add ':' to the front of the parameter names since PDO doesn't either. Take a look at the PDO::prepare function for an example here:
http://php.net/manual/en/pdo.prepare.php

Revision history for this message
Don Schoeman (don.sch) wrote :

Sorry, just a small typo, this line:
formatSQL($sql, array(':foo' => 1, ':bar' => 'test'field a, ':baz' => 'test field b')

should be:
formatSQL($sql, array(':foo' => 1, ':bar' => 'test field a', ':baz' => 'test field b')

Revision history for this message
Greg (gregfr) wrote :
Revision history for this message
Don Schoeman (don.sch) wrote :

Well, yes, that is also an example. Just so we are on the same page though, I'm not saying we should support Prepared statements right now as that is part of PDO. I'm saying we should support the standard method of parameterizing queries, e.g. using :name instead of %(name)s.

Revision history for this message
TitanKing (titan-phpdevshell) wrote :

What Don means is that both INSERT INTO sometable (foo, bar, baz) VALUES (:foo, ':bar', ':baz') and %(name)s be supported. I think the time has come to move from the mysql connector to the PDO connector as this support such features already, rewriting an abstraction layer while an existing compiled version is already available is waistful resources. I also think we need to move as many Query PHP methods over to call compiled PDO methods rather as in general should be much faster than how we can handle and manipulate query results in PHP.

I will be creating a wishlist bug report for this and assign Don to deal with the PDO connector implimentation which was started by Greg. The situation Don is in, is currently favoratible for PHPDevShell as high strain complicated and fast queries is required for his project. This will allow him continual testing and benchmarking of the different scenarios.

I too have been working and trying out various PDO related things in the past and it was determined back then, that this is the way to go forward. Although I assign the completion of this development and documentation cycle to Don, this will be a multi development initiative.

Revision history for this message
Greg (gregfr) wrote :

Is this one a duplicate of the PDO one? if so, we can close it

Revision history for this message
Don Schoeman (don.sch) wrote :

I'd like to keep this one open so that when I start working on this issue specifically we can keep track of it separately from the rest of the PDO work.

Revision history for this message
Greg (gregfr) wrote :

ok I change the status and keep it open

Changed in phpdevshell:
importance: Wishlist → Low
status: New → In Progress
Revision history for this message
TitanKing (titan-phpdevshell) wrote :

Just wondering if we made any progress with supporting the ':foo' convention?

Revision history for this message
Don Schoeman (don.sch) wrote :

Unfortunately not. Once we switch over to PDO then this convention will automatically be supported. Of course all the queries will have to be re-written to support this then.

Greg (gregfr)
tags: added: query
removed: queries
Revision history for this message
Greg (gregfr) wrote :

I think this should be explicitly activated. PDO syntax is too limited to be the default one.
However, we could consider adding an implicit conversion so that the good python syntax could be use with prepared statements.

Revision history for this message
Greg (gregfr) wrote :
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.