Problem/Motivation

#2183231: Make ContentEntityDatabaseStorage generate static database schemas for content entities changed the schema of file_managed.uri to have a prefixed unique key, i.e. a unique key with a length.

This is problematic as

  1. this is currently not supported on PostreSQL: #2278493: Drop support for unique keys specified as an array of column name and length (breaks PG installation)
  2. files that start with the same characters but are different in the end can no longer be inserted into the database

Proposed resolution

Swap the prefixed unique key for a proper unique key.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#1 postresql-schema.patch1.32 KBtstoeckler
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler’s picture

sun’s picture

The unique key can't be the only validation. Don't we have a uniqueness validation for the URI in PHP code, too?

If we do, why don't we simply change the unique index into a regular index?


FWIW, Postgres's limitation is quite reasonable. Limiting the length of a unique key is a bit illogical... But I guess that rather belongs into #2278493: Drop support for unique keys specified as an array of column name and length (breaks PG installation)

tstoeckler’s picture

So I didn't actually try it, but I'm pretty sure that it's not being checked in PHP. At least I couldn't find anything remotely related.

We should really have a Unique constraint which we can just slap on various field definitions.

mradcliffe’s picture

Manually tested patch on postgresql 9.3.4. No unique index issues encountered during installation though the installation did not complete due to another issue, which I will will hunt down and file/find.

Ninja edit: Looks like shortcut's setting a serial column to 0.

2014-06-14 17:46:42 UTC ERROR:  setval: value 0 is out of bounds for sequence "shortcut_id_seq" (1..9223372036854775807)
2014-06-14 17:46:42 UTC STATEMENT:  SELECT setval('shortcut_id_seq', GREATEST(MAX(id), '0')) FROM shortcut
mradcliffe’s picture

Welp. I think the static database schemas mega patch causes all database storage entities to try to set their initial id as 0 (#2183231: Make ContentEntityDatabaseStorage generate static database schemas for content entities).

mradcliffe’s picture

Sorry for the comment spam. I did not realize that #2279395: The PostgreSQL backend does not handle NULL values for serial fields gracefully covered that already.

stefan.r’s picture

plach’s picture

Assigned: plach » Unassigned

The change makes sense to me: the current field schema definition allows for a max size of 2048 characters, but previously the uri column had a max size of 255, so this should not cause problems during a migration from D7.

If we want to address the unique constraint here then this is NW otherwise RTBC.

Edit: if we want to allow for bigger URIs then also @sun's suggestion of turning the unique constraint into a regular index makes perfect sense to me.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

OK, thanks. Marking RTBC then, as this breaks PGSQL which we still do support ATM.

Making the URI non-unique would be a change in behavior - or otherwise the uniqueness would have to be validated in PHP, thus would rely on such a UniqueConstraint. Let's explore that in a follow-up.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 6140e65 and pushed to 8.x. Thanks!

  • Commit 6140e65 on 8.x by alexpott:
    Issue #2279363 by tstoeckler: Fixed file_managed.uri should not specify...

Status: Fixed » Closed (fixed)

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