I cannot create the number of desired fields because of the size of views_display.display_options (BLOB, normal). I'd to suggest to enlarge the size of display_options to big and use the column type TEXT instead of BLOB (consistent with views_object_cache).

Patch following asap.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex_b’s picture

Status: Active » Needs review
FileSize
1.48 KB

This patch addresses the problem I encountered - is this going in the right direction?

merlinofchaos’s picture

Be sure to find the instances of db_encode_blob and db_decode_blob. pgsql is going to explode on this. :/

merlinofchaos’s picture

Status: Needs review » Needs work
smscotten’s picture

Priority: Normal » Critical

This patch fixed my issue, although I had to increment views_update_6005 to views_update_6007 to accommodate the 6.x-2.6 version. I'm guessing that is going to land me in trouble sometime down the line.

I don't understand why this is even necessary. Why is a BLOB limited to 64K? That seems like a rather low limit.

Considering that attempting to exceed 64K of display_options deletes all the fields in a view without any warning or error message, I'm calling this a critical bug. Even if the issue is not fixed, error messages should be thrown instead of trashing (probably simply corrupting) data.

It may be unusual for a view to require 80 fields, each with custom labels, but it's hardly an edge case.

davej’s picture

I've just been bitten by this, too. If changing the type from blob to text causes complications that delay the fix then, given that the size rather than the type is what causes the problem, can we just up the size and get the fix in? I agree that it's critical.

To fix my immediate problem, would it be safe to manually alter table and change the blob to a mediumblob (16M limit) ?

Dave

smscotten’s picture

Category: task » bug
Status: Needs work » Needs review

Yes, it's critical, and being held up in "needs work" because the description of the problem is vague and very few of us know enough about the specifics of Postrgres to either fix it or declare the "brokenness" invalid. Why Postgres would "explode" because of using a text field is beyond my capability to even imagine.

Is it that db_decode_blob will "explode" if it tries to decode some other kind of field? If so, why doesn't it "explode" on the varchars and ints in that same table as load_row looks at the schema?

If the problem with this patch can't actually be described or explained so that someone can at least attempt to fix it, this should not be "needs work."

Also changing from "task" to "bug report" as the "task" was to fix a bug.

merlinofchaos’s picture

Splicer: If you want to handle the bug reports that occur when it breaks on postgres, then feel free to mark this okay. Go ahead and commit it. Since you don't even understand what I'm talking about, I feel confident you're not going to be able to respond to those issues.

Sadly, I will still have to if it gets committed.

Also, it is not at all helpful to be snarky at me over it. You will find that treating people who work on volunteer software like employees does not really actually get anything done.

I'll leave this 'needs review' for now, but the reality is, it needs work.

davej’s picture

I manually changed the blob to a mediumblob and it solved the problem for me, on MySQL 5.0.77 .

@Splicer, @merlinofchaos: people's frustration over a technical problem can sometimes come across as disrespect for those who contribute their time and expertise. I take it that we are united in our goal of making good open source software better - whether in a small way by reporting bugs and testing patches or through active development - and in our appreciation of the creators and maintainers of this software.

@merlinofchaos: if you could clarify whether it is the change of size, of type or both that causes complications with postgres, I think that would help to move this issue forward.

Thanks,

Dave

smscotten’s picture

@merlinofchaos Yes, I know I'm totally ignorant about all things Postgres. I haven't even had a machine with a Postgres install since 1999. But in all my googling and searching here on drupal.org, I found no indication that using text fields will cause Postgres to explode. So I can only assume that there is something else going on that you're referring to, but which you don't want to explain or even name. Hence my frustration. I don't think of you as an employee, I think of you as someone who walked in, looked at a solution to a real problem, said "that won't work" and walked out without another word.

I'm happy to put my time and effort into a solution that will work better, but I don't have time for the wild goose chase. If you're going to tell alex_b, me, and anyone else that wants this fixed that we have to do more work (aka "needs work"), yes, I do think some indication as to what that work is actually needed would be in order. "Will explode" is so vague I don't know how you can expect anyone else to take action on it.

For the record, I do not want to cause you more work. "Needs review" means "come look and see if there's something wrong" and "needs work" means "we're waiting on whoever created the patch to revisit the issue." I am genuinely interested in knowing what is wrong with this solution. Sadly, there is no status for "postponed (person that proposed the fix needs more info from the person who said the fix needs work)" so I didn't set to that status. I apologize for having little faith in you to come back and tell us what the problem is, but if the status is "needs review" then maybe someone else will find the problem and actually tell us what it is.

merlinofchaos’s picture

Well, I thought I was fairly clear. We're changing a blob to a text in the patch. That means instances of db_decode_blob and db_encode_blob will be wrong. MySQL ignores these functions, so on MySQL you'll never notice the problem. And more or less, I was waiting on the original author to come fix the patch. :/

smscotten’s picture

There are no instances of db_encode_blob in Views. Do you know of any places where it is called indirectly? Otherwise that's 50% down, 50% to go.

There are two instances of db_decode_blob, both of which appear to run against every field in the table views_display, including several non-blob fields. Why isn't it already blowing up? Furthermore, it begs the question: what is *ever* getting decoded if the Views module is never encoding anything with db_encode_blob?

All db_decode_blog seems to do is call pg_unescape_bytea. From what I can tell by reading, pg_unescape_bytea will throw an error on miscast data fed to it in PostgreSQL 7.2.0 and 7.2.1. Drupal 6 requires PostgreSQL 7.4 or higher. So is this really still a problem? Or have my attempts at investigation gone in completely wrong directions?

smscotten’s picture

@merlinofchaos, will you answer one or more of the following questions so that this issue haqs a chance of being resolved someday?

Is db_encode_blob called anywhere? I see it in database.inc and database.xxx.inc but grepping my entire drupal directory I see nothing that calls db_encode_blob.

If the problem is that db_decode_blob can be called on something that hasn't been encoded with db_encode_blob, but db_encode_blob is never called, isn't that a bigger problem?

Am I wrong that db_decode_blob is already being called on every column in the table views_display? If so, is calling db_decode_blob already blowing up on all the other fields?

Is the problem you refuse to describe to us related to miscast data being fed to pg_unescape_bytea? Is there actually a problem on PostgreSQL 7.4 or higher as required by Drupal 6?

Instead of switching from TEXT to BLOB, would changing the patch here to switch from BLOB to MEDIUMBLOB address your issue?

If all the above is irrelevant, would you be so kind as to take a moment to be more specific than "will be wrong"?

merlinofchaos’s picture

AT this point I think it's been established that I've been overly cautious about the text/blob thing.

That said, this can't go in until http://drupal.org/node/531686 is resolved, because views_schema_1() should be locked and not modified. Committing things like this without that fixed will make that even harder.

merlinofchaos’s picture

Status: Needs review » Needs work

Ok, #531686: views_update_6000 causes errors is in, so this patch needs to be updated for that.

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.23 KB

So here is a version, i just ported the patch, i didn't tryed out the patch yet.

merlinofchaos’s picture

Version: 6.x-2.x-dev » 7.x-3.x-dev
Status: Needs review » Patch (to be ported)
FileSize
3.48 KB

I made a minor change to this to make it not duplicate the field, and committed to 2.x and 3.x. Because the base .install update has not been committed to 7.x, this needs to be ported to 7.x. THe actual patch I committed is attached. Please note that I integrated update 6008 into Views 2 in this same patch, so it probably will take a little extra effort to get it to apply properly.

dawehner’s picture

One general questions, should this kind of updates be moved to 700X, because perhaps some people are using views on d7 already.

merlinofchaos’s picture

Since the databases are exactly the same, let's keep them precisely in sync as long as possible

dawehner’s picture

are the 600X updates also be runned for 7.x? I don't know this. If yes, this is fine.

merlinofchaos’s picture

Yes, the numbering is merely convention. The current linear numbering is also problematic, since you can update from 6.x to 7.x at any point along the way, meaning you have to consider what happens if someone updates to 7.x when they were at 6000, 6001, 6002, 6003 etc.

So as long as they remain identical, we should just keep them in sync to save trouble. In fact, if we are making changes in the D7 schema that wouldn't harm D6, we should probably make them in the D6 schema too just to keep things linear until we can't anymore.

dawehner’s picture

Status: Patch (to be ported) » Fixed

and fixed.

Thanks for the informations!

smscotten’s picture

Sweet. Thank you, Merlinofchaos!

Status: Fixed » Closed (fixed)

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

roderik’s picture

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 in his reservations about db_encode_blob / db_decode_blob "exploding". At least in some cases, things stopped working.

There are two instances of db_decode_blob, both of which appear to run against every field in the table views_display, including several non-blob fields. Why isn't it already blowing up?

Because this isn't true. db_decode_blob() was only running against serialized fields. Which were only blob fields (but are now also text fields).

Patch attached FWIW.