Zend Framework

QuoteInto doesn't work with Sqlite

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.0.0 RC2
  • Fix Version/s: 1.5.2
  • Component/s: Zend_Db
  • Labels:
    None
  • Fix Version Priority:
    Should Have

Description

We discover a problem with quoteInto and SQLite.

The Zend Framework generate this kind of SQL (we use quoteInto for projects_id and users_id):

DELETE FROM "usvn_users_to_projects" WHERE (projects_id = '1' AND users_id = 1)

But in SQlite quote seems to be mandatory and we need write:

DELETE FROM "usvn_users_to_projects" WHERE (projects_id = '1' AND users_id = '1')

We made an ugly hack to fix the problem into Zend/Db/Adapter/Pdo/Sqlite.php

public function quote($value)
{
    if (is_int($value) || is_float($value)) {
       return parent::quote("$value");
    }
    return parent::quote($value);
}

Activity

Hide
Bill Karwin added a comment -

I'm not sure I understand. Are the SQL datatypes of projects_id and users_id integer or varchar?

Does SQLite have a problem comparing an integer literal to a varchar? Some RDBMS brands have the opposite problem, you can't compare a string literal to an integer column.

You also have a workaround:

$expr = $db->quoteInto('users_id = ?', (string) 1);
Show
Bill Karwin added a comment - I'm not sure I understand. Are the SQL datatypes of projects_id and users_id integer or varchar? Does SQLite have a problem comparing an integer literal to a varchar? Some RDBMS brands have the opposite problem, you can't compare a string literal to an integer column. You also have a workaround:
$expr = $db->quoteInto('users_id = ?', (string) 1);
Hide
Bill Karwin added a comment -

This is not a critical bug, since there are already two workarounds.

I'd be glad to fix it regardless, but I'm reducing the severity.

Show
Bill Karwin added a comment - This is not a critical bug, since there are already two workarounds. I'd be glad to fix it regardless, but I'm reducing the severity.
Hide
Bill Karwin added a comment -

Resolved in revision 5314.

Show
Bill Karwin added a comment - Resolved in revision 5314.
Hide
Bill Karwin added a comment -

Reopening. SQLite does require that integers are compared to integers, and strings are compared to strings.

Show
Bill Karwin added a comment - Reopening. SQLite does require that integers are compared to integers, and strings are compared to strings.
Hide
Bill Karwin added a comment -

Fixed in revision 5493.

Show
Bill Karwin added a comment - Fixed in revision 5493.
Hide
Julien Duponchelle added a comment -

Problem is still present. That's why we use this patch:
=== www/Zend/Db/Adapter/Pdo/Sqlite.php
==================================================================
— www/Zend/Db/Adapter/Pdo/Sqlite.php (revision 1088)
+++ www/Zend/Db/Adapter/Pdo/Sqlite.php (local)
@@ -278,4 +278,22 @@
return $sql;
}

+ /**
+ * Safely quotes a value for an SQL statement.
+ *
+ * If an array is passed as the value, the array values are quoted
+ * and then returned as a comma-separated string.
+ *
+ * @param mixed $value The value to quote.
+ * @param mixed $type OPTIONAL the SQL datatype name, or constant, or null.
+ * @return mixed An SQL-safe quoted value (or string of separated values).
+ */
+ public function quote($value, $type = null)
+ {
+ if (is_int($value) || is_float($value)) { + return parent::quote("$value", $type); + }
+ return parent::quote($value, $type);
+ }
+
}

Show
Julien Duponchelle added a comment - Problem is still present. That's why we use this patch: === www/Zend/Db/Adapter/Pdo/Sqlite.php ================================================================== — www/Zend/Db/Adapter/Pdo/Sqlite.php (revision 1088) +++ www/Zend/Db/Adapter/Pdo/Sqlite.php (local) @@ -278,4 +278,22 @@ return $sql; } + /** + * Safely quotes a value for an SQL statement. + * + * If an array is passed as the value, the array values are quoted + * and then returned as a comma-separated string. + * + * @param mixed $value The value to quote. + * @param mixed $type OPTIONAL the SQL datatype name, or constant, or null. + * @return mixed An SQL-safe quoted value (or string of separated values). + */ + public function quote($value, $type = null) + { + if (is_int($value) || is_float($value)) { + return parent::quote("$value", $type); + } + return parent::quote($value, $type); + } + }
Hide
Bill Karwin added a comment -

Unmark fixed version.

Show
Bill Karwin added a comment - Unmark fixed version.
Hide
Wil Sinclair added a comment -

This issue should have been fixed for the 1.5 release.

Show
Wil Sinclair added a comment - This issue should have been fixed for the 1.5 release.
Hide
Wil Sinclair added a comment -

Please categorize/fix as needed.

Show
Wil Sinclair added a comment - Please categorize/fix as needed.
Hide
Wil Sinclair added a comment -

This doesn't appear to have been fixed in 1.5.0. Please update if this is not correct.

Show
Wil Sinclair added a comment - This doesn't appear to have been fixed in 1.5.0. Please update if this is not correct.
Hide
Simon Mundy added a comment -

Hi Julian

I'm looking into this for the 1.6 release if it's able to be resolved. Can I clarify - what is the reason for explicitly casting as a string by default? The only concern I have defaulting to a string type is that it will break an existing expected behaviour. Or is SQLite expecting a string and this behaviour has always been incorrect.

The patch itself is straight-forward, as are the tests, but I want to be sure this is absolutely required before committing.

Show
Simon Mundy added a comment - Hi Julian I'm looking into this for the 1.6 release if it's able to be resolved. Can I clarify - what is the reason for explicitly casting as a string by default? The only concern I have defaulting to a string type is that it will break an existing expected behaviour. Or is SQLite expecting a string and this behaviour has always been incorrect. The patch itself is straight-forward, as are the tests, but I want to be sure this is absolutely required before committing.
Hide
Julien Duponchelle added a comment -

I just test and now it's work without our patch. Thanks for your help.

Show
Julien Duponchelle added a comment - I just test and now it's work without our patch. Thanks for your help.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:

Time Tracking

Estimated:
1d
Original Estimate - 1 day
Remaining:
1d
Remaining Estimate - 1 day
Logged:
Not Specified
Time Spent - Not Specified