RoleTest and RoleCheckoutTest fail with Uncaught PHP Exception Drupal\Core\Database\IntegrityConstraintViolationException: "SQLSTATE[23502]: Not null violation: 7 ERROR: null value in column "rpid" violates not-null constraint: INSERT INTO simpletest713714uc_roles_products (pfid, rpid, nid, model, rid, duration, granularity, by_quantity, shippable, end_override, end_time) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10); Array

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

TR’s picture

Component: Code » Tests
TR’s picture

This appears to be a problem with db_merge() not working on PostgreSQL when the primary key is a serial field. We use essentially the same code for both roles and file downloads, since they are both file features, which is why these tests are the ones that fail in PostgreSQL.

This seems to be a problem in core - all the db_ functions EXCEPT db_merge are tested against tables with serial and non-serial primary keys, but db_merge() is only tested in the case of non-serial primary keys. db_merge() is the recommended replacement for drupal_write_record() which we used to use in D7 for roles and files, but drupal_write_record() has always had this known problem with PostgreSQL and serial primary keys. There have been many issues about this, but they have all been closed and "fixed" in core by not using db_merge() when core has a serial primary key, thereby avoiding the issue instead of addressing it.

Short of a core fix for db_merge(), we'll probably just have to test the key and do a conditional db_insert()/db_update(). Yuck, but it will work.

TR’s picture

Issue tags: +beta blocker

Let's do this for beta.

TR’s picture

TR’s picture

TR’s picture

TR’s picture

TR’s picture

Another wild guess as to what may be going wrong ...

  • TR committed 018495f on 8.x-4.x
    Issue #2625022 by TR: Make tests pass on PostgreSQL
    
TR’s picture

Success! PostgreSQL tests are now passing for the first time!

Lessons learned:

  1. db_merge() does not work on PostgreSQL with serial primary keys. Normally, you would assemble an array of fields to write, one of which was the primary key. If the key was null, then db_merge() would call db_insert() to create a new record. If the key was set, db_merge() would call db_update() to modify the record. This works for non-serial keys, and works well on MySQL with serial keys, but on PostgreSQL with serial keys if the key is null then you CAN'T pass it to db_merge() as one of the fields, while if it's not null then you MUST pass it. In other words, on PostgreSQL you must pass different arguments to db_merge() depending on what the key value is. Because you have to test this, you will already know whether you need to call db_insert() or db_update(), so it's more readable and cleaner to just explicitly call db_insert() or db_update() yourself. The only utility of db_merge() is to have the same code whether you're doing an update or an insert, but that can't work with PostgreSQL. This was needed to fix the uc_role and uc_file failures.
  2. SELECT doesn't guarantee an order for returned records. Although the order will always be the same with a given database, using another database such as PostgreSQL is likely to (and in fact DOES) give you a different order. So if your tests rely on the order of records, be sure to add an ORDER BY clause to your SELECT. This was needed to fix the uc_fulfillment failure, where the packages returned by PostgreSQL were listed in the opposite order than with MySQL, so deleting the "first" package gave different results with the different databases.
longwave’s picture

Nice work. The first one feels like a bug in the Postgres driver - maybe it should be reported against core? The second one isn't strictly a bug, more an assumption that succeeds in MySQL but is not guaranteed in other databases.

TR’s picture

The first one feels like a bug in the Postgres driver - maybe it should be reported against core?

Yes it should, I just don't know if I have the energy to take on what is probably a multi-year task of trying to get that fixed in core.

Status: Fixed » Closed (fixed)

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