Compare and sync with database.mysql

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Cvbge’s picture

Priority: Normal » Critical
Status: Active » Needs work
FileSize
5.57 KB

First draft. database.pgsql review is complete, but still need to compare with 4.6 and write update functions.

Cvbge’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
12.33 KB

1. merged http://drupal.org/node/38551
2. adds CREATE UNIQUE INDEX to prefix.sh
3. changes totalcount from int to bigint to prevent overflow
4. the rest: unifies index names, syncs with database.mysql

Dries’s picture

-1 on the bigint. int can be 2.147.483.647. If you use unsigned int, it can be 4.294.967.295. That ought to be enough not?

The aggregator tables have URLs too. aggregator_feed has both a link and a url. aggregator_item has a link. Do these need to be updated? That said, virtually any field can overflow in Drupal. I'm not sure that switching everything from varchar to text is the right approach. I suggest not to merge both patches for now.

Cvbge’s picture

Maybe I should have mentioned, that the changes are to postgresql only.

Mysql already uses bigint. Maybe this should be changed to unsigned int then? Postgresql does not have unsigned types, so max int is 2147483647. Whether this is enough depends on how often can a node be viewed daily.

E.g. if there's a frontpage node that is displayed when entering a site, then to last at least 2 years, it'd have to be displayed less then 2.941.758. So I think we can assume the counter will last for 2 years at least. That's enough IMO. When cvs comes back I'll change the patch not to change this to bigint. What to do with mysql?

About aggregator - after quick code scan it looks like the fields are populated from a form, so the administrator would have to enter too long url (and the form probably will stop him). OTOH, the watchdog/accesslog overflow can be easily triggered by anonymous user by simply accessing the page.

Cvbge’s picture

This patch does not touch totalcount anymore. Leaves as integer for postgresql (should be noted, again, the mysql uses bigint).

No other changes.

If aggregator tables need similar varchar->text change, then I'd prefer to do it in separate issue, as this patch was tested and is big enough as it is.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)