Hi,

The cTools cache functionnalities doesn't work with postgres.
As reported by the Drupal Community (http://drupal.org/node/322756), new lines in text fields are encoded with special characters so you must use php db_decode_blob function before unserializing.
So in ctools_object_cache_get in object-cache.inc on line 40, db_decode_blob should be called before unserialize this way:

$cache[$key] = unserialize(db_decode_blob($data->data));

Hope this helps.

CommentFileSizeAuthor
#15 495240-15.patch2.57 KBroderik
#1 decode_blob.patch1.58 KBmikl
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mikl’s picture

mikl CreditAttribution: mikl commented
Status: Active » Needs review
FileSize
decode_blob.patch1.58 KB

Here's a patch for the mentioned case and a similar one in export.inc.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

I am confused.

This field is not a blob field. My experience in the past that running db_decode_blob on a field that is not, in fact, a blob causes errors. Since I can't test this, I am unsure that htis is the right answer.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Hi, I'am using drupal on POSTGRES as well, and PANELS 6.x-3.0-rc1. I had the same problem. Impossible for me to use PANELS at all, it either gave an error as described, or simply didn't keep the content defined.

Just changed these two lines as reported by vinzentt and mikl (thanks a lot to you two!!):

In ctools\includes\object-cache.inc:
//$cache[$key] = unserialize($data->data);
$cache[$key] = unserialize(db_decode_blob($data->data));

In ctools\includes\ export.inc:
//$object->$field = empty($info['serialize']) ? $data->$field : unserialize($data->$field);
$object->$field = empty($info['serialize']) ? $data->$field : unserialize(db_decode_blob($data->$field));

And everything seems to run smooth now. Please take note merlinofchaos, it'd be great if this issue could be corrected from rc1 for the postgres users. Let me know if you need my help for testing.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Version: 6.x-1.0-beta3 » 6.x-1.0-rc1
Status: Needs review » Patch (to be ported)
sdboyer’s picture

sdboyer CreditAttribution: sdboyer commented
Status: Patch (to be ported) » Needs review

(Not the right status for this issue, switching it back.)

Thanks for the additional report, zimbo. Earl, I'm finding convincing the explanation vinzentt originally gave - postgres needs blob decoding when there are newline characters - and given that that function does nothing on mysql(i), I'm leaning towards committing this. Do you remember any of those cases you were recollecting before where db_decode_blob() was harmful?

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

AS I remember it, if you try to run db_decode_blob, in pgsql, on a field that is not actually a blob, it corrupts the data.

groklem’s picture

groklem CreditAttribution: groklem commented

As far as I can tell this patch should work.

In Postgres db_decode_blob is just a wrapper for pg_unescape_bytea()
pg_unescape_bytea just converts octal strings (\\110 is converted to 'H' etc) to binary and preserves regular ascii.

eg.
drupal >> \d pg_unescape_bytea('\\110\\145\\154\\154\\157 world');
(string) Hello world

I think that as long as the content does not contain data that pg_unescape_bytea could mistake for binary data then this patch should be ok.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented
Status: Needs review » Reviewed & tested by the community

Fixed the module on my site with no errors.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented
Status: Reviewed & tested by the community » Needs work

The change in export.inc is causing errors.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented
Status: Needs work » Reviewed & tested by the community

False alarm. I was using a pre-decoded string in the body of a custom content pane.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Committed.

I am still nervous about this, but it looks like we'll go out with this.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Just to give some more feedback regarding Drupal on POSTGRES. I just updated from rc1 versions to ctools 6.x-1.0 and panels 6.x-3.0 and everything is running OK. I just replaced the complete module folders, and all the content I had published before is still there working perfectly.

sdboyer’s picture

sdboyer CreditAttribution: sdboyer commented

Thanks for keeping us apprised. Please do let us know if anything goes sideways.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
CreditAttribution: roderik as a volunteer commented
Issue summary: View changes
Related issues: +#2687401: [core] Sessions don't work on PostgreSQL after updating to Drupal 6.38
FileSize
495240-15.patch2.57 KB

I'm not going to open a new issue anymore (just refer to this issue from the open Core/PostgreSQL issue #2687401: [core] Sessions don't work on PostgreSQL after updating to Drupal 6.38 because who knows, I may be the last person running D6 on PostgreSQL.

But for the record: merlinofchaos was right and db_decode_blob() should not be called on text fields. I saw some issues with Ctools/Panels/... while testing my sites before an upgrade (though I haven't detailed exactly which issues, exactly when).

I think the likely original bug that caused this issue to be opened, is that ctools_object_cache_set() treats the text field as a blob (and therefore calls db_encode_blob(), which it shouldn't be doing) on insert.

Attached patch

  • Iterates on hunk #1 in patch #1 _ctools_export_unpack_object(): only calls db_decde_blob() on blob fields.
  • Undoes the patch to ctools_object_cache_get() and instead patches ctools_object_cache_set().
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
CreditAttribution: roderik as a volunteer commented
Related issues: +#865142: Make ctools_object_cache.data DB column blob for storing arbitrary bytes.

Oh wow, the web of issues is more convoluted than I thought. For completeness:

While my posted patch** fixes the code (i.e. 'unfixes' the wrong fix in ctools_object_cache_get() and does the real fix in does the real fix in ctools_object_cache_set() so that serialized strings don't end up in the database unnecessarily-encoded) for the case when ctools_object_cache.data is a TEXT field...

...there's also an open issue to turn ctools_object_cache.data back into a BLOB field because of bugs. (The earlier change from BLOB to TEXT was apparently based on a misunderstanding, AFAICT.) That would make this newest patch partly unnecessary.

I'm not going to post more patches because I don't need that change personally. I think. I just summarized all the things #865142-7: Make ctools_object_cache.data DB column blob for storing arbitrary bytes..

** that is: the changes to object-cache.inc are the actual fixes which would be 'overruled' by #865142. The two generalizations in export.inc are still good potential fixes to handle any kind of text-or-blob fields on all database types. (I just haven't seen an actual bug in the wild which is fixed by this. I think.)