Cleans up a couple "lazy" sql statements in aggregator. Most "enterprise" (commercial, enterprise, large, whatever taxonomy you wanna use) will not allow a group by on a large-data column type such as a CLOB/BLOB, etc.

Most of the original sql statements had SELECT table.* , and then spelled out the columns in the group by clause. Well, the SELECT * includes the "image" column in this case.

Kind of makes sense...I mean, why would you need to group on a BLOB type column where every value is unique?

CommentFileSizeAuthor
aggregator_sql_clean_up.patch1.79 KBSouvent22

Comments

Souvent22’s picture

Souvent22 commented
Status: Active » Needs review
chx’s picture

chx commented
Title: SQL Clean Up (Enterprise Friendly) » Aggregator should not use GROUP BY on a blob
Version: x.y.z » 6.x-dev
Category: bug » task
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

Patch still applies, and I agree with spelling out the fields you're selecting, however:

1. It's inconsistent to put the query on more than one line (I know it's .= so internally it's still on one line, but it still breaks consistency).
2. Doesn't this mean you can no longer display feed images in the aggregator view?

Souvent22’s picture

Souvent22 commented

No, just means you'll have to pull them in speratley; which this should be a big deal anyways as if you pull in the link, the users browser will cache such small images anyway; and we can implement out own caching if need be on our end if the extra db calls really become a big deal.

dpearcefl’s picture

dpearcefl commented

Is there any interest in this issue any more?

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.