I recently updated the search_api module to 7.x-1.8. I am not able to save any nodes(old or new) on my site since then. I am getting the message

The website encountered an error.

Logs showed this error

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'item_id' cannot be null: INSERT INTO {search_api_et_item}

I was able to trace it down to 

/**
 * Implements hook_node_access_records_alter().
 *
 * Marks the node as "changed" in indexes that use the "Node access" data
 * alteration. Also marks the node's comments as changed in indexes that use the
 * "Comment access" data alteration.
 */
function search_api_node_access_records_alter(&$grants, $node) {
  foreach (search_api_index_load_multiple(FALSE) as $index) {
    $item_ids = array();
    if (!empty($index->options['data_alter_callbacks']['search_api_alter_node_access']['status'])) {
      $item_id = $index->datasource()->getItemId($node);
      $item_ids = array($item_id);
    }
    elseif (!empty($index->options['data_alter_callbacks']['search_api_alter_comment_access']['status'])) {
      if (!isset($comments)) {
        $comments = comment_load_multiple(FALSE, array('nid' => $node->nid));
      }
      foreach ($comments as $comment) {
        $item_ids[] = $index->datasource()->getItemId($comment);
      }
    }

    if ($item_ids) {
      $indexes = array($index->machine_name => $index);
      search_api_track_item_change_for_indexes($index->item_type, $item_ids, $indexes);
    }
  }
}

The line $item_id = $index->datasource()->getItemId($node); is returning NULL for all my nodes for the default_multilingual_node_index from the Search API Entity Translation module.

CommentFileSizeAuthor
#6 2715115-6--getItemId.patch672 bytesdrunken monkey
#4 search_api-2715115-4.patch758 bytesgaëlg

Comments

nidaismailshah created an issue. See original summary.

nidaismailshah’s picture

nidaismailshah commented
Issue summary: View changes
gaëlg’s picture

gaëlg
he/him
Primary language French
Location Lille, France
commented

I face the same bug. The code you highlighted was introduced on 04/20, by http://cgit.drupalcode.org/search_api/commit/search_api.module?id=91df71...
I'm on it, should be easy to fix.

gaëlg’s picture

gaëlg
he/him
Primary language French
Location Lille, France
commented
Status: Active » Needs review
StatusFileSize
new search_api-2715115-4.patch758 bytes

There might be cases where $index->datasource()->getItemId($node) returns NULL while it shouldn't, but anyway, there are cases where it's right to return NULL, according to the doc:

/**
* Retrieves the unique ID of an item.
*
* @param mixed $item
* An item of this controller's type.
*
* @return mixed
* Either the unique ID of the item, or NULL if none is available.
*
* @throws SearchApiDataSourceException
* If any error state was encountered.
*/
public function getItemId($item);

So that I added a check to avoid adding NULL in the $item_ids array.

nidaismailshah’s picture

nidaismailshah commented
Status: Needs review » Reviewed & tested by the community

Seems to work fine for me.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Title: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'item_id' cannot be null: INSERT INTO {search_api_et_item} » Fix getItemId() to always return an ID
Project: Search API » Search API Entity Translation
Version: 7.x-1.18 » 7.x-2.x-dev
Component: General code » Code
Priority: Critical » Major
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2715115-6--getItemId.patch672 bytes

I'm amazed that we currently allow NULL as the return value for getItemId(). That doesn't sound like a good idea. (We already changed it in the D8 version.)
I'm pretty sure the correct way to fix this would be to fix the search_api_et module to always return an item ID for an entity. It really just seems like laziness that they don't already do that, from how the code looks.
Applying this fix here would lead to the same security issue that was fixed with that hook implementation to pop up again for multilingual node indexes. (Also, if we really want to apply that fix, we at least should also apply it for comments.)

Status: Needs review » Needs work

The last submitted patch, 6: 2715115-6--getItemId.patch, failed testing.

thepanz’s picture

thepanz commented

I am still unsure about latest patch, in which case the $item would be missing the `search_api_et_id`?
Could it be related to a dirty data in the index?
Do you think it would be correct to throw the SearchApiDataSourceException exception?

nico.knaepen’s picture

nico.knaepen commented

We also encountered issues with getItemId returning NULL. Patch seems to work fine.
Also a memory allocation error on entity save, which occured before, doesn't occur anymore.

thepanz’s picture

thepanz commented

@nico.knaepen thank you for your feedback.
Could you check that the search functionality still works correctly? Are nodes correctly retrieved during a search, regarding their language?

minoroffense’s picture

minoroffense commented

The null return value also causes a chain of events to trigger a reindex of all items within a given index. We have a site with 7000 nodes and it triggers an entity load on each to get the en, fr and und values to be indexed.

idebr’s picture

idebr commented
Status: Needs work » Needs review

  • fe94494 committed on 7.x-2.x 
    Issue #2715115 by GaëlG, drunken monkey: Fix getItemId() to always...
idebr’s picture

idebr commented
Status: Needs review » Fixed

Saving a node with search_api_et enabled no longer triggers a fatal error after #2742053: Node access records change implementation could pass wrong parameters to trackItemChange was committed in Search API. However, this change still makes sense to commit.

Status: Fixed » Closed (fixed)

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