WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19387 closed defect (bug) (fixed)

post_content_filtered should be longtext

Reported by: ejdanderson Owned by: ryan
Milestone: 3.4 Priority: normal
Severity: normal Version: 1.5
Component: Database Keywords: has-patch
Focuses: Cc:

Description

From my understanding, the post_content_filtered column exists to cache expensive content transformations. Thus, its type should match that of post_content - 'longtext' instead of just 'text'.

Attachments (1)

longtext_post_content_filtered.diff (1.5 KB) - added by ejdanderson 3 years ago.
$wpdb->posts not wp_posts

Download all attachments as: .zip

Change History (13)

comment:1 follow-up: @nacin3 years ago

Agreed in theory. In practice, this kind of ALTER might get ugly.

comment:2 @johnbillion3 years ago

I wonder if anyone actually uses that field to cache processed post content.

comment:3 @nacin3 years ago

I've used it during content migrations. Mark implemented it for Markdown: http://wordpress.org/extend/plugins/markdown-on-save/.

comment:4 in reply to: ↑ 1 @ejdanderson3 years ago

Replying to nacin:

Agreed in theory. In practice, this kind of ALTER might get ugly.

Ugly in terms of run time on a DB such as WP.com? Or the 3 additional bytes per row required?

Replying to johnbillion:

I wonder if anyone actually uses that field to cache processed post content.

I've used it to store XML defined by an additional DTD.

comment:5 follow-up: @nacin3 years ago

I was referring to run time, yes.

comment:6 in reply to: ↑ 5 @ejdanderson3 years ago

Replying to nacin:

I was referring to run time, yes.

Thanks for clearing that up, I have a strong feeling you have more experience with that than I do :)

Consequently, wouldn't it be better to get this in as early as possible to minimize the size of the DBs it needs to preform the alter on? Or is it a fix that more than likely won't make it into core at any time?

Last edited 3 years ago by ejdanderson (previous) (diff)

comment:7 @nacin3 years ago

  • Version changed from 3.3 to 1.5

Consequently, wouldn't it be better to get this in as early as possible to minimize the size of the DBs it needs to preform the alter on?

There are quite a few installs out there already. :-)

Our existing database schema API doesn't really allow the flexibility of doing this for new installs only, without touching existing installs. (Which is something that would be easier to swallow.)

I do know it took them about 3 weeks on WP.com to add the commentmeta tables. I don't expect them to ever merge over a change like this one, so it's irrelevant to them, but it's still something to consider. I'm not familiar with the performance aspects here. Someone like ryan who was around during previous schema changes could talk about it a bit more.

comment:8 @ejdanderson3 years ago

  • Cc ejdanderson@… added
  • Keywords has-patch added; needs-patch removed

@ejdanderson3 years ago

$wpdb->posts not wp_posts

comment:9 @markjaquith3 years ago

I got bitten by this issue recently while using a plugin that uses post_content_filtered. We should do the ALTER. It's crazy that it doesn't match post_content, given its intended usage by plugins.

comment:10 @nacin3 years ago

Simply changing it in $wp_queries will be enough for dbDelta() to run the ALTER, so we don't need the separate ALTER in the upgrade routine.

That also means we can't actually apply this only to new installs, and not old installs. I think that will be okay, but we should look into how slow this kind of ALTER is depending on the size of the post table. (Does it help that this field will nearly always be empty?)

comment:11 @ryan3 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [19863]:

Switch post_content_filtered from text to longtext so that it matches post_content. Props ejdanderson. fixes #19387

comment:12 @helenyhou3 years ago

  • Milestone changed from Awaiting Review to 3.4
Note: See TracTickets for help on using tickets.