Using ShURLy with PostgreSQL generate this error:

PDOException: SQLSTATE[42601]: Syntax error: 7 ERROR: syntax error at or near "source" LINE 1: SELECT rid, destination FROM shurly WHERE BINARY source = 's... ^: SELECT rid, destination FROM {shurly} WHERE BINARY source = :q AND active = 1; Array ( [:q] => shurly ) in shurly_boot() (line 179 of /usr/local/drupal/sites/all/modules/shurly/shurly.module).

The cause is due to the use of BINARY, which allows to make a case sensitive query. In PostgreSQL (and SQLite), queries are, by default, case sensitive.

Removing the BINARY syntax resolve this issue.

However, i don't know how to make a good patch for this. I mean, do we need to make a condition for database other than Mysql ? I don't know.

Comments

makangus’s picture

Status: Active » Needs review
StatusFileSize
new1.45 KB

How about using a binary column? The Schema API says
'binary': A boolean indicating that MySQL should force 'char', 'varchar' or 'text' fields to use case-sensitive binary collation. This has no effect on other database types for which case sensitivity is already the default behavior.

It seems like it should make MySQL case sensitive and leave PostgreSQL and SQLite alone.

makangus’s picture

Added a hook update to the patch.

jibus’s picture

Hi makangus !

Thanks you very much for your reply and patch.

Tested and it works under PG !

I notice that there is still a BINARY statement aroud:

  // we add 'BINARY' to ensure that we're doing a CASE SENSITIVE query
  $query = "SELECT * FROM {shurly} WHERE BINARY source = :short";
  if ($check_active) {

l874-876

makangus’s picture

Good catch. How about this?

jibus’s picture

Tested #4,

Everything looks fine: creation / reports / delete OK

Thanks for you quick reply !

jibus’s picture

Status: Needs review » Reviewed & tested by the community

One week later, still no problem !

Set this to RTBC

jibus’s picture

Is is possible to commit this :) ?

jibus’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Patch (to be ported)

Committed against 7.x-1.x thanks ! Need to reroll for 6.x

cs_shadow’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new1.88 KB

Patch in #4 backported for 6.x-1.x

jibus’s picture

StatusFileSize
new1.83 KB

The Drupal 6 API for changing field is a different from 7. We need to drop primary key and indexes in order to update the field.

Updated patch.

jibus’s picture

StatusFileSize
new1.81 KB

Reroll patch, need only to remove indexes from the field. Forgot to set the ouput.

jibus’s picture

StatusFileSize
new1.91 KB

Update patch, missing field definition.

jibus’s picture

StatusFileSize
new1.93 KB

Sorry again, this one should be OK.

jibus’s picture

Status: Needs review » Needs work

Ok, my knowledge in Drupal 6 Schema was not helpful

Binary is not available as spec. Need to run a manual query

jibus’s picture

Status: Needs work » Needs review
StatusFileSize
new2.13 KB

Updated patch.

jibus’s picture

StatusFileSize
new2.14 KB

Hope this will be the last one :)

jibus’s picture

Status: Needs review » Closed (fixed)

Committed against 6.x-1.x

Thanks !