PHP warnings using db_insert('actions_aid')->execute() in actions_save()

That's my bad on the original patch for DBTNG & actions.inc. :) Since this is no longer just a short DB command, we should maybe chain everything down so it looks like:

db_insert('actions_aid')
  ->useDefaults(...)
  ->execute();

Also, I hate to be nit-picky, but please use the already-coined term 'DBTNG' and not 'TNGDB' so that issues searches for 'DBTNG' can find your patches. Thanks!

Other than those two small issues, patch applies cleanly and actions/trigger tests are at 100% pass with 476 passes. Should be RTBC after a quick revision.

This was not caught by our tests? ==> we need to extend them

No, as currently it does not fail. This patch depends on another patch.

After a little more investigating, the actions_aid is a table with one auto-increment field, that's it. What's buggy about not specifying the value for an auto-increment field? It does not need to have ->useDefaults(array('id')). Since there are no other fields so db_insert('actions_aid') should be valid. We don't use ->useDefaults(array('id')) in our database tests:

  /**
   * Test that we can run a query that is "default values for everything".
   */
  function testDefaultInsert() {
    try {
      $query = db_insert('test')->useDefaults(array('job'));
      $id = $query->execute();

      $schema = drupal_get_schema('test');

      $job = db_query("SELECT job FROM {test} WHERE id = :id", array(':id' => $id))->fetchField();
      $this->assertEqual($job, $schema['fields']['job']['default'], t('Default field value is set.'));
    }
    catch (Exception $e) {
      $this->assertTrue(FALSE, $e->getMessage());
    }
  }

  /**
   * Test that we can insert fields with values and defaults in the same query.
   */
  function testDefaultInsertWithFields() {
    try {
      $query = db_insert('test')->fields(array('name' => 'Bob'))->useDefaults(array('job'));
      $id = $query->execute();

      $schema = drupal_get_schema('test');

      $job = db_query("SELECT job FROM {test} WHERE id = :id", array(':id' => $id))->fetchField();
      $this->assertEqual($job, $schema['fields']['job']['default'], t('Default field value is set.'));
    }
    catch (Exception $e) {
      $this->assertTrue(FALSE, $e->getMessage());
    }
  }

What would be a safe SQL way to insert a blank row for a table that has defaults for all fields? INSERT INTO mytable VALUES ()? Maybe the db_insert should check if there are no fields specified, then use that syntax. I'll stop chiming in until I get a more authoritative answer.

So, summary so far: Steps to reproduce the problem:
- Go to admin/settings/actions
- Create a custom action like 'Redirect to URL' and save it
- Following warnings should show on the result page:
* warning: array_fill() [function.array-fill]: Number of elements must be positive in /home/davereid/Projects/drupal-head/includes/database/mysql/query.inc on line 66.
* warning: implode() [function.implode]: Bad arguments. in /home/davereid/Projects/drupal-head/includes/database/mysql/query.inc on line 67.

We have two alternatives to fix this:
1. Use db_insert('actions_aid')->useDefaults(array('aid'))->execute()
2. Fix the database insert query to work around an insert query with no specific or default fields defined, but on a table that has default values for all it's fields
3. Cause error if no fields are defined as proposed in #321100: Empty insert statements should fail gracefully

Doing solution #2 seems the most reasonable to me since this can help avoid the problem in the future with little to no change to the database API. We shouldn't have to specifically have to use ->useDefaults(...) on an auto-increment field. Ideally, I'd like to finish my work on integrating the aid back into the actions table and remove the one-field actions_aid table, but we should have a fix in case this comes up in the future.

Attached is a patch that does a quick count() check on the InsertQuery->defaultFields in InsertQuery_driver::__toString() exactly like we already do with InsertQuery->insertValues. It also includes a test (thanks hwong3i) for the db_insert('mytable')->execute() statement in the Database Insert, default fields tests.

looks like #329223: Notices in ActionsConfigurationTestCase might be a duplicate of this.