skinr-ui uses a three-parameter concat function:
$result = db_select('block', 'b')
->fields('b')
->distinct()
->where('CONCAT(b.module, \'-\', b.delta) = :identifier', array(':identifier' => $form['skinr']['sid']['#value']))
->range(0, 1)
->execute();
Now, it would be possible to hop on over to the skinr queue and ask them to split the form value and do a two-clause where
instead; however, as a postgres user, this seems sort of like an uphill battle. Unless Drupal is going to forbid three-parameter CONCAT functions (and how would that policy be communicated?), it seems better to support this in core so that we don't have to keep chasing these down whenever they pop up.
The attached patch defines an update function and modifies the pgsql install function to define a new three-argument CONCAT function. Using anynonarray for all three parameters rather than trying to hit all permutations of text and anynonarray seems preferable, and works in the current instance.
To reproduce: On a Drupal-7 install using postgres, install and enable skinr and skinr-ui with some theme that uses it (fusion-starter is good). Mouse over the "configure" widget and select "edit skin". Without this patch, you get the following error:
PDOException: SQLSTATE[42883]: Undefined function: 7 ERROR: function concat(character varying, unknown, character varying) does not exist LINE 4: WHERE (CONCAT(b.module, '-', b.delta) = 'search-form') ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts.: SELECT DISTINCT b.* FROM {block} b WHERE (CONCAT(b.module, '-', b.delta) = :identifier) LIMIT 1 OFFSET 0; Array ( [:identifier] => search-form ) in block_skinr_preprocess_hook_callback() (line 94 of /srv/www/d7.com/sites/all/modules/skinr/modules/block.skinr.inc).
Apply this patch and run updatedb, and the above operation works fine (edit skin dialog is displayed).
Workaround: Waiting for this patch to land? Change your working directory to your site's configuration folder (containing settings.php) and run the following drush command:
drush sqlq "CREATE OR REPLACE FUNCTION \"concat\"(anynonarray, anynonarray, anynonarray) RETURNS text AS 'SELECT CAST(\$1 AS text) || CAST(\$2 AS text) || CAST(\$3 AS text);' LANGUAGE 'sql';"
This has the same affect as the attached patch, but of course is temporary in nature (must be done once per d7 site). Workaround does not cause problems with the update script in this patch.
Comment | File | Size | Author |
---|---|---|---|
#18 | reroll-pgsql-concat-args-1072322-18.patch | 12.88 KB | Percy101 |
#17 | reroll-pgsql-concat-args-1072322-7.patch | 12.88 KB | Percy101 |
#7 | pgsql-concat-args-1072322-7.patch | 12.83 KB | jaredsmith |
#6 | pgsql-concat-three-args-1072322-6.patch | 2.03 KB | jaredsmith |
drupal-pgsql-concat.patch | 1.66 KB | greg.1.anderson | |
Comments
Comment #1
greg.1.anderson CreditAttribution: greg.1.anderson commentedAdded #1073108: Use of three-parameter concat function is bad for postgres compatibility to skinr queue for documentation / fallback.
Note also that the above simpletest pass was done on MySQL only, which this patch does not affect (good that the test passed to confirm this!). Seems like the postgres automated tests have been broken for a while.
Comment #3
Crell CreditAttribution: Crell commentedThis would be a feature request, not a bug report, I think. We don't make any claims currently to support SQL functions in a portable fashion.
Is 3-part-concat currently supported in both MySQL and SQLite? If we're going to try and support it, it would have to be consistently supported across all core databases. I'm not sure what the implications would be for the contrib drivers, either.
DamZ's input would be helpful here.
Comment #4
chx CreditAttribution: chx commentedWell, almost every database we support core or contrib uses
||
(MySQL in ANSI mode too and we run in ANSI). The exception is MS SQL and there does not seem to be an in-database, cross-database solution. Do we want to add a concat class with a __toString?Comment #5
Liam MorlandComment #6
jaredsmith CreditAttribution: jaredsmith commentedI've made a patch that handles concat with three arguments. If you'd like, I'd be happy to extend the patch to cover cases of four, five, and even six arguments if there's a use for it.
Comment #7
jaredsmith CreditAttribution: jaredsmith commentedAttached is a more complete patch, which covers three, four, and five arguments.
Comment #9
jhedstromThis needs an updated patch, as well as an IS update since it's been so long and the examples are about D7.
Comment #17
Percy101 CreditAttribution: Percy101 as a volunteer commentedComment #18
Percy101 CreditAttribution: Percy101 as a volunteer commentedI am sorry but the above reroll and this one has the same content. But the above reroll that is in comment number 17 does not follow the right nomenclature.
Comment #25
quietone CreditAttribution: quietone at PreviousNext commentedCleaning up tags.