Menu router does "too many SQL variables" on SQLite resulting in silent nondisplay of menu items

Hmm, limit of 999 is hardcoded.
This seems tough.

Crosslinking an old noisy issue which suggests this issue may also appear when doing crud operations on a big count of entities: #1210092: PDOException: SQLSTATE[HY000]: General error: 1 too many SQL variables: SELECT t.* FROM {field_data_body} t WHERE (entity_type =

I think we have 3 levels of concern here:
* A menu router which breaks in nonspecial circumstances on SQLite. Raising prio as i suppose we want to target sqlite.
* A PDO driver which might want to magically circumvent this limit. If that's possible at all i suppose that's a tough one and should be a separate issue.
* Adding a warning to PDO developer docs that arrays in conditions do work but can hit DB limits.

In fact there are 2 queries that break the limit.

I'm wondering if filtering in PHPland helps here and did a quick profiling on a D7 site:

Filtering in DBland: 647 items
Before query #1: 0.0030994415283203 ms / 0.12 kb
After query #1: 30.982971191406 ms / 157.016 kb
Before query #2: 31.01110458374 ms / 157.216 kb
After query #2: 68.348169326782 ms / 157.264 kb
Finished: 68.377017974854 ms / 157.456 kb

Filtering in PHPland: 647 items
Before query #1: 0.0030994415283203 ms / 0.12 kb
After query #1: 1.0020732879639 ms / 6.816 kb
Before query #2: 1.0371208190918 ms / 3.672 kb
After query #2: 18.75901222229 ms / 2581.376 kb
Finished: 25.389194488525 ms / 15.72 kb

So in PHPland we are way faster with a temporary memory footprint worse by some MB. (Gut feeling worst case: 6MB for 2000 items) Sounds like a reasonable deal. Here's the code for reference. I'll roll this in a patch after getting some sleep...

  $profiling = array('Filtering in DBland: ' . count($menu) . ' items');
  $profiling_time = microtime(TRUE);
  $profiling_memory = memory_get_usage();
  $profiling[] = 'Before query #1: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  $paths = array_keys($menu);
  // Updated and customized items whose router paths are gone need new ones.
  $result = db_select('menu_links', NULL, array('fetch' => PDO::FETCH_ASSOC))
    ->fields('menu_links', array(
      'link_path',
      'mlid',
      'router_path',
      'updated',
    ))
    ->condition(db_or()
      ->condition('updated', 1)
      ->condition(db_and()
        ->condition('router_path', $paths, 'NOT IN')
        ->condition('external', 0)
        ->condition('customized', 1)
      )
    )
    ->execute();
  $profiling[] = 'After query #1: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  foreach ($result as $item) {
    // As $menu may be huge and hit PDO limits we filter out paths in PHPland rather than in DB.
    //if (!$item['updated'] && array_key_exists($item['router_path'], $menu)) {
    //  continue;
    //}
    $router_path = _menu_find_router_path($item['link_path']);
    if (!empty($router_path) && ($router_path != $item['router_path'] || $item['updated'])) {
      // If the router path and the link path matches, it's surely a working
      // item, so we clear the updated flag.
      $updated = $item['updated'] && $router_path != $item['link_path'];
      db_update('menu_links')
        ->fields(array(
          'router_path' => $router_path,
          'updated' => (int) $updated,
        ))
        ->condition('mlid', $item['mlid'])
        ->execute();
    }
  }
  $profiling[] = 'Before query #2: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  // Find any item whose router path does not exist any more.
  $result = db_select('menu_links')
    ->fields('menu_links')
    ->condition('router_path', $paths, 'NOT IN')
    ->condition('external', 0)
    ->condition('updated', 0)
    ->condition('customized', 0)
    ->orderBy('depth', 'DESC')
    ->execute();
  $profiling[] = 'After query #2: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  // Remove all such items. Starting from those with the greatest depth will
  // minimize the amount of re-parenting done by menu_link_delete().
  foreach ($result as $item) {
    // As $menu may be huge and hit PDO limits we filter out paths in PHPland rather than in DB.
    //if (array_key_exists($item->router_path, $menu)) {
    //  continue;
    //}
    _menu_delete_item($item, TRUE);
  }
  $profiling[] = 'Finished: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  watchdog('menu_router_profile', implode('<br>', $profiling));

  $profiling = array('Filtering in PHPland: ' . count($menu) . ' items');
  $profiling_time = microtime(TRUE);
  $profiling_memory = memory_get_usage();
  $profiling[] = 'Before query #1: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  //$paths = array_keys($menu);
  // Updated and customized items whose router paths are gone need new ones.
  $result = db_select('menu_links', NULL, array('fetch' => PDO::FETCH_ASSOC))
    ->fields('menu_links', array(
      'link_path',
      'mlid',
      'router_path',
      'updated',
    ))
    ->condition(db_or()
      ->condition('updated', 1)
      ->condition(db_and()
        //->condition('router_path', $paths, 'NOT IN')
        ->condition('external', 0)
        ->condition('customized', 1)
      )
    )
    ->execute();
  $profiling[] = 'After query #1: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  foreach ($result as $item) {
    // As $menu may be huge and hit PDO limits we filter out paths in PHPland rather than in DB.
    if (!$item['updated'] && array_key_exists($item['router_path'], $menu)) {
      continue;
    }
    $router_path = _menu_find_router_path($item['link_path']);
    if (!empty($router_path) && ($router_path != $item['router_path'] || $item['updated'])) {
      // If the router path and the link path matches, it's surely a working
      // item, so we clear the updated flag.
      $updated = $item['updated'] && $router_path != $item['link_path'];
      db_update('menu_links')
        ->fields(array(
          'router_path' => $router_path,
          'updated' => (int) $updated,
        ))
        ->condition('mlid', $item['mlid'])
        ->execute();
    }
  }
  $profiling[] = 'Before query #2: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  // Find any item whose router path does not exist any more.
  $result = db_select('menu_links')
    ->fields('menu_links')
    //->condition('router_path', $paths, 'NOT IN')
    ->condition('external', 0)
    ->condition('updated', 0)
    ->condition('customized', 0)
    ->orderBy('depth', 'DESC')
    ->execute();
  $profiling[] = 'After query #2: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  // Remove all such items. Starting from those with the greatest depth will
  // minimize the amount of re-parenting done by menu_link_delete().
  foreach ($result as $item) {
    // As $menu may be huge and hit PDO limits we filter out paths in PHPland rather than in DB.
    if (array_key_exists($item->router_path, $menu)) {
      continue;
    }
    _menu_delete_item($item, TRUE);
  }
  $profiling[] = 'Finished: ' . ((microtime(TRUE) - $profiling_time)*1000)  . ' ms / ' . ((memory_get_usage() - $profiling_memory)/1000) . ' kb';
  watchdog('menu_router_profile', implode('<br>', $profiling));

Patch flying in. 7.x for testbot.

Same for 8.x.

Urps, forget #3.

OK and here's the improved 8.x version.
Maybe in the 2nd query we can get filtering out of phpland with some join but that's up to the who deeply grokks the menu system.

+++ b/core/includes/menu.incundefined
@@ -2919,13 +2919,19 @@ function _menu_navigation_links_rebuild($menu) {
+    if (array_key_exists($item->router_path, $menu)) {

Uh, this can't work because an entityquery just returns IDs. We need "select id, router_path"...
Need to grokk new EFQ.

Yes we should do that anyway and i rolled that in #2031261: Make SQLite faster by combining multiple inserts and updates in a single query.

BUT that patch alone does not solve the issue, the exception from the issue summary is still thrown.
So it looks we're back at #8.

Out of curiosity i used the code from #2 to profile #2031261: Make SQLite faster by combining multiple inserts and updates in a single query:

Without that patch:

Filtering in PHPland: 1167 items
Before query #1: 0.0040531158447266 ms / 0.12 kb
After query #1: 2.065896987915 ms / 6.824 kb
Before query #2: 2.1259784698486 ms / 3.648 kb
After query #2: 52.454948425293 ms / 4501.024 kb
Finished: 74.632883071899 ms / 19.824 kb

With that patch:

Filtering in PHPland: 1167 items
Before query #1: 0.0028610229492188 ms / 0.12 kb
After query #1: 1.4150142669678 ms / 6.784 kb
Before query #2: 1.4510154724121 ms / 3.664 kb
After query #2: 33.931016921997 ms / 4501.144 kb
Finished: 47.703981399536 ms / 19.824 kb

Which shows that that patch speeds up query #2 from 50ms to 32ms.

dunno. re-tested it on a different install and still get exception.

can you check the patch in #2031261-2: Make SQLite faster by combining multiple inserts and updates in a single query if i got the right place for the option.

for the records: here's a simple php snippet to trigger the exception:

$query = db_select('users', 'u')
  ->fields('u', array('uid'))
  ->range(0, 1);

$range = range(1,1000);

$query->condition('u.uid', $range, 'not in');
$result = $query->execute()->fetchAll();

EDIT: a range of integers does not raise the exception here. see #18

i'm such a moron. forget #12 and #15.
after killing the fcgi process #2031261: Make SQLite faster by combining multiple inserts and updates in a single query in fact solves this issue.

leaving this open for reference and prioritizing the other issue.

Updated issue summary.

I was WRONG thinking that the snippet in #16 raises the exception,
so i was WRONG that this issue is fixed by #2031261: Make SQLite faster by combining multiple inserts and updates in a single query.
It depends if we have string or int placeholders.

@damien: here's the proof that PDO::ATTR_EMULATE_PREPARES does not help, maybe it's broken in PDO sqlite.

* aplied patch from #2031261: Make SQLite faster by combining multiple inserts and updates in a single query
* running this code in devel/php raises the "too many sql variables" exception

/* 
Tried all these ways to tell PDO Sqlite driver to emulate prepared statements:
- patched constructor (issue 2031261 )
- $connection->setAttribute()
- $query options
- DatabaseStatement_sqlite::query() options
Still we get "too many sql variables" exception.
*/
$connection = Database::getConnection();
$connection->setAttribute(PDO::ATTR_EMULATE_PREPARES, TRUE);

$options = array(PDO::ATTR_EMULATE_PREPARES => TRUE);

$query = db_select('users', 'u', $options)
  ->fields('u', array('uid'))
  ->range(0, 1);
// strval is esential here to raise the exception. an array of ints will not.
$range = array_map('strval', range(10,2000));
$query->condition('u.uid', $range, 'not in');
$result = $query->execute()->fetchAll();

Remarkable that only an array of strings raises the exception.
(EDIT: ah, i see: sqlite has some hack in \DatabaseStatement_sqlite::getStatement() where is replaces numeric placeholders itself.)

So the general solution is worked on over there: #2031261: Make SQLite faster by combining multiple inserts and updates in a single query

Rite!