Book module tests for postgres error

Unable to reproduce on my localhost installation which uses MySQL (although mine is not a new site and there are probably some other minor differences).

However it looks as though there is no default value defined for field {node_revisions.log} - see http://api.drupal.org/api/file/modules/node/node.install/6/source. Maybe MySQL uses empty string, whereas PgSQL throws an error.

Do you get the initial error with other content types? (This is the important error, since it prevents the node from being properly created and so the DB ends up with a mangled node/node revision entry.) I can't see anything much specific to book pages that would restrict your error to them.

>This problem does not occur in Drupal 5.7
That's because there was separate code for PgSQL that specified the field as log text NOT NULL default ''. I suspect this may be an old bug that has been reintroduced with the new database schema API.

>I am pretty sure that this error happened at least with pages too
Would be useful if you could confirm as this would mean 6.0 basically does not work witn pgSQL.

>how to recover?
It won't happen once the install code for the {node_revisions} table is fixed, if that is indeed the problem. In your case you would need as a minimum to delete the entry from the {node} table that corresponds to the node id of the page, and all entries from the {node_revisions} table with this node id. You may even be able to do this via the usual admin/content interface. Also if you created a menu item and/or book outline entry for the page then they would need to go as well. Potentially there are other links to other tables as well but hopefully you've not created any of those at this stage!

[Update:]
I think the critical thing which leads to this bug is that on the admin/content/node-type/book screen, under Workflow settings, "create new revision" is *not* checked. Therefore you don't get a "Log message" box on the node input/edit form, and as a result $node->log is not set, hence not even an empty string for $node->log is inserted in the DB.

Thanks,

>Creating and editing ordinary pages worked
Even when the "create new revision" box is unchecked on the content type admin page? If the Log message box is present on the create/edit form then this problem should not arise, so if you are operating as a user with "administer content" permissions I think you always get that box, hence should never have a problem. Can you confirm what happens creating an ordinary page with permissions set such that this box is *not* present?

>As long as you have "none" selected for book outline one can edit as long as one wants
Don't see how this fits with your permissions listed above for the "new" user - which show no permissions for book module. Previously you said "outline does not matter (wether new book or none)" - i.e. you always get the error. Sorry to nitpick, just trying to home in on the problem! Also I think that editing is not where the problem arises - it's when creating a new book page that the original problem occurred AFAICS.

>Also books work, when "create revisions" is set
At least that is what I would have expected! :-D

I have the same druus's problem.
Posgresql 8.2.6, php 5.2.4.

OK this tiny patch adds a default value of empty string for {node.log} in hook_scheme() in node.install.

As noted above at #3, in Drupal 5.x the field is created as log text NOT NULL default '' for Postgresql (but log longtext NOT NULL for MySQL) - see http://cvs.drupal.org/viewvc.py/drupal/drupal/modules/system/system.inst.... For some reason the default '' got removed from the MySQL definition at revision 1.48, but no issue is referenced so I'm not sure why.

Especially since I only have MySQL I've not had time to trace back and work out why the problem only seems to happen in the situation described, but pls see if this patch helps.

Patch should also apply just fine to HEAD.

I play a little with node_install_log_field_223820.patch and it seems OK.

I'm glad it worked, however I fear it may not be an appropriate fix in the long term - I've just discovered that default values for text fields are not good news (specifically, MySQL strict mode doesn't allow them - see e.g. http://drupal.org/node/201427#comment-678943).

So I think the correct fix would be to ensure that $node->log is always set before the node is saved. Since I've not reproduced the error on my own setup, it will be a bit tricky for me to track down the error at source and fix it. Hoping someone else will come along and advise/muck in ..!

Update: see also:
Adding NOT NULL columns with no default - http://drupal.org/node/159329
MySQL peculiarities with implicit defaults - http://drupal.org/node/159330

We are not going to modify the database schema in Drupal 6 and in this case, it would not be a good solution anyway. We added code before to ensure that the log will be set. Look into node_save(), there is a huge code block just to ensure this:

    // When inserting a node, $node->log must be set because
    // {node_revisions}.log does not (and cannot) have a default
    // value.  If the user does not have permission to create
    // revisions, however, the form will not contain an element for
    // log so $node->log will be unset at this point.
    if (!isset($node->log)) {
      $node->log = '';
    }

Now it needs to be looked into, why this is not reached for you.

(Marking as no-patch, as the proposed patch was completely off).

When editing a page for non-administrators, the book module force the saving of a revision in nodeapi($op='presave'). In that case, $node->log is not set.

The attached patch move the code highlighted in #12 by Gábor to _node_save_revision(), and always adds $node->log when inserting a new revision.

However, it feels like we should teach drupal_write_record() to set default values to NOT NULL text fields when they are not set.

Minor point - can I suggest changing

    // When inserting a node, $node->log must be set because
    // {node_revisions}.log does not (and cannot) have a default
    // value.  If the user does not have permission to create
    // revisions or if a module forced the creation of a new revision,
    // however, the form will not contain an element for log
    // so $node->log will be unset at this point.

to

    // When inserting a node revision, $node->log must be set
    // because {node_revisions}.log does not (and cannot) have a
    // default value. If the user does not have permission to
    // create revisions then the form will not contain an element
    // for log so $node->log will be unset at this point. But when
    // creating a new node or if a module forces it, these users
    // do actually end up creating a new revision.

For the record, d6-core-book-empty-log.patch also works here.

Bugs get fixed in 7.x then backported.

Here is an updated patch, for D6 and D7, with minor changes to comments.

This bug basically breaks the book module for PostgreSQL users, and as such should get fixed as soon as possible.

#238388: Editing a book page by another user issues pgsql ERROR: null value in column "log" violates not-null was a duplicate.

Tried on a fresh install with postgresql. I can't replicate the error message, but I do get the page not found with an alias (however, accessing the node with node/3 does work). I need to do more debugging and I don't want to mix other issues into this one, but it seems like there must be some issue with the path module also related to the original problem.

Here is a test case for this bug.

It fails on my PostgreSQL 8.2 installation, but not on MySQL, as expected.

With the patch (reroll against HEAD included), all tests succeed.

PS: note that PostgreSQL tests currently return a lot of PHP exceptions because of an unrelated patch (http://drupal.org/node/256001).

Forget the patch (the test is still valid), I put together a more general solution in #261258: Fix node_save() insertion logic.

I think this should probably go in tests now.

In that case, we have a patch (tests-223820-core-book-empty-log.patch in #20).

Still applies with offset, we shouldn't concatenate prefixes onto randomName() though.

marked http://drupal.org/node/424742 as a duplicate.

This looks pretty trivial to fix?

Moving this out of the critical bugs queue - see #607038: Meta issue: fix gaps in code coverage.

Is patch for tests still needed? another issue already commited #261258: Fix node_save() insertion logic

A lot of work went into psql for D7 in the past 24 months.

This has for sure been resolved in Drupal core 7.94, book tests are all passing green now. All tests green, have a look here:

PHP 8.2 & pgsql-13.5 2,110 pass tested on commit

Please upgrade your core to Drupal core 7.94.