Postgres bug: wrong data type in query parameters (Improve query builder to use field information)

Well, this is og module (which includes a notifications plug-in)

Note to og maintainers: For the rest of the Notifications modules, we are always using '%s' parameters which seem to work fine with both pgsql and mysql. That notifications_fields.value is a varchar field, it can hold both numbers, strings, etc...

Notifications builds this query using the following block in notifications_get_subscriptions:

  foreach ($params as $field => $value) {
    $name = strstr($field, '.') ? $field : 's.'. $field;
    $where[] = is_numeric($value) ? $name .' = %d' : "$name = '%s'";
  }

The is_numeric test will return true for both is_numeric(1) as well as is_numeric("1"). is_int (and conversely is_string) will not.

I've gone ahead and committed to HEAD and 5-3 using a string cast. However, this doesn't solve the issue as such.

Leaving as active for now.

-K

I noticed the "subscription fields" option in hook_notifications which appears to allow type definitions for each field. However, I don't see this ever being used by the notifications module. If this is the way to go, please let me know.

-K

@Zen,

You're right that's currently not used anywhere. It's a 'forward looking' feature, we'll be using that information to optimize the queries still more and to automatically fix all these query conditions. Now we have integer and string values in notifications_fields, we can do string matching or integer matching speeding up the queries a bit more.

So though not needed yet, it is advisable to implement it for all plug-ins to be forward compatible.

About this bug, it looks like it belongs to Notifications module.

So, the deal is: you implement 'subscription fields' in og_notifications, we fix the query builder to use that information when building the query.

As a side effect, this should improve performance too :-)

This is already implemented. Queries now should use the right data type, either from schema or from 'subscription fields' definition.

This should also improve performance, as we are using int conditions when available instead of string matching.