Lines 237, 257 and 284 of schema.test attempt to create fields that are not-null and have no default value. I can investigate tomorrow if no one beats me.

This is causing a fail in SQLite and I think it is also causing one in Postgres, not sure.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dmitrig01’s picture

Status: Active » Needs review
FileSize
1.82 KB

Fixes that issue (pgsql is different). There is still one fail on sqlite for SchemaTestCase, but it fails with or without the patch (oddly, it fails on my computer but nt on ci.drupal.org)

dmitrig01’s picture

FileSize
1.66 KB

This patch doesn't need tests, as the tests already exist, they fail, and this fixes them. There is another fail in the same test case that's being terribly hard to debug as it only fails my local machine and not ci.drupal.org, and it's very obscure. But unrelated to this patch -- I think there's something wrong with the test.

dmitrig01’s picture

btw, the only difference between #2 and #1 is one line comment change

dmitrig01’s picture

Title: Tests attempt to create not-null fields with no default values » SQLite ignores initial values
Component: simpletest.module » sqlite database

Also to note, most of this code was copy+pasted from mysql driver.

Damien Tournoud’s picture

Title: SQLite ignores initial values » Complete the implementation of DatabaseSchema::addField() for SQLite
FileSize
6.29 KB

#2 made the schema test pass on SQLite, but it is not exactly the proper way to do that.

We can add a field directly to a SQLite table using ALTER TABLE if and only if: it is not a PRIMARY KEY, it is not both NOT NULL and without a default value (more precise constraints are described in http://www.sqlite.org/lang_altertable.html). In all the other cases, we have to fallback to the slower method of creating a new table and migrating the data over.

To simplify, I considered that we use the slower method whenever we have to add indexes or we have a NOT NULL column without a default value. I implemented the rest of the DatabaseSchema::addField() and that passes tests on ci.

dmitrig01’s picture

I read over the code, tried it out and ran the test - looks good. I'd RTBC it but I'm ineligible.

Damien Tournoud’s picture

A few improvements from chx: we don't need to workaround bugs in addField() and addExpression() anymore, they have been long fixed. Also fixed a comment typo.

Damien Tournoud’s picture

And fixed a small typo in changeField().

chx’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome!!

Committed to HEAD. :D

Status: Fixed » Closed (fixed)

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