SqlBase throws fatal error when extended directly

Patch which forces the fetch type on the statement sent to the migrate iterator.

very strange... just a super simple source causes this.

Test failures btw are a test violating the database interface to mock results into an iterator. Not an actual failure but a troublesome tests.

use Drupal\migrate\Plugin\migrate\source\SqlBase;

/**
 * ...
 *
 * @MigrateSource(
 *   id = "basic_migration",
 *   source_module = "doc_migration"
 * )
 */
class Docs extends SqlBase {

  /**
   * {@inheritdoc}
   */
  public function fields() {
    return [
      'name' => 'Title',
      'description' => 'Description',
    ];
  }

  /**
   * {@inheritdoc}
   */
  public function getIds() {
    return ['id' => ['type' => 'string']];
  }

  /**
   * {@inheritdoc}
   */
  public function query() {
    return $this->getDatabase()
      ->select('other_docs', 'docs')
      ->fields('docs', ['id', 'name', 'description']);
  }

}

So resolved _why_ it works for commerce. The "solution" is in SqlBase::select() but here's the problem. Neither query() nor select() have any documented requirement on the state of the statement they return but we've implemented a requirement that they setup the query options with a fetch mode. (query isn't even documented at all other then a return type so double bad.)

I don't think this is a documentation problem though. This setup smells. Most other interact with the result set from the query, the developer will fetch from one of the many fetch methods on the statement that exist explicitly to solve the fetch mode issue. Think fetchAssoc. The only reason we have this particular issue is we're trying to be clever/efficient and using the iterator behavior of the statement. That is fine but we need to enforce the fetch mode where we build the result set explicitly instead of relying on an implied behavior of other methods.

Thinking about this more, specifically because the developer doesn't have the ability to modify SelectInterface, this could exclude a use case where the developer consumes a select query from some other query builder to populate their source. If that query builder has an object fetch mode the developer would have to do some sort of really complicated munging or entirely rebuild the query, or replace initializeIterator which is a very large complicated bit of code they should probably not touch. All bad options.

So... SqlBaseTest::testHighWater makes no assertions and doesn't really test anything. The only reason phpunit hadn't warned us about it is a random statement calls a mock method that counts as an assertion but has nothing to do with highwater.

I was tempted to delete the whole test but since we're not actually asserting anything about the $query_result this should work and is less invasive.