Document that SqlBase source plugin must use the ignore_map configuraition option if expressions are used in the query

https://stackoverflow.com/questions/11685310/mysql-using-inner-joins-on-...

Sql error: Unknown column 'foo' in 'on clause'

I want to work on the wording, but we should include why ignore map exists (slows performance, but improves SQL validity in certain cases). And we should point off to the SQL handbook if that works in the overall flow of things. On my phone, so I'll work on that later.

ignore_map: (optional) Source data is joined to the map table by default. If set to TRUE, the map table will not be joined. Set to FALSE if the sql has an expression added to it which will result in invalid SQL. Mapping helps speed up migration performance, but if it results in invalid SQL, then just disable it to retain valid SQL.

The conditional_expr used with ON is any conditional expression of the form that can be used in a WHERE clause.

https://dev.mysql.com/doc/refman/en/join.html

Standard SQL disallows references to column aliases in a WHERE clause. This restriction is imposed because when the WHERE clause is evaluated, the column value may not yet have been determined.

https://dev.mysql.com/doc/refman/en/problems-with-alias.html

An example of invalid query when ignore_map: true is set:

  public function query() {
    $query = $this->select('node', 'n')
      ->fields('n', ['nid']);
    // Add the value for the custom id.
    $query->addExpression(':now', 'current__date', [':now' => date('Ymd')]);
    return $query;
}

I'm much more +1 on your version. I just wanted to get all the docs together in a single place so we have them as a reference.

ignore_map: (optional) Source data is joined to the map table by default to improve migration performance. If set to TRUE, the map table will not be joined. If the SQL query uses expressions, the ignore_map option must be set to TRUE because expressions result in column aliases in a JOIN clause; which is considered invalid SQL.

This seems like additional clarity on the docs. +1 on RTBC, but I wrote good parts so I'll leave to someone else.

I took a look at \Drupal\Core\Database\Query\Select and I think that we can make this even better. I do think we should document it, but we should also take advantage of Select::getExpressions() and automatically set ignore_map to TRUE if there are any expressions attached to the query. Then we can simply document that behavior.

Something like this might work:

$ignore_map = (bool) $query->getExpressions();

I feel like someone could write an addExpression() query that results in valid SQL in a JOIN. I'm not sure how to articulate that thought without making it so techno-bable and conditional that it has zero meaning.

I did some digging, and it does seem possible. The other tricky thing, this whole thing is only important if the value in the expression is also part of the source id in getIds(). If it isn't part of the IDs, then it doesn't matter about the ignore_map.

    $query = $this->select('node', 'n')
      ->fields('n', ['nid']);
    // Add the value for the custom id.
    $query->addExpression('n.uid'); // <= I think this is valid and won't blow up because it isn't a dynamic value and is joinable.
    return $query;

I think this is closer what I'd want to say. We have the key words in there of invalid SQL and joins. So hopefully if someone runs into invalid SQL on the join, they can figure out to try setting ignore_map to true.

Since none of the wording is really at all what I wrote, I think I can now mark RTBC.

One tiny thing, then +1 to RTBC.

+++ b/core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
@@ -24,8 +24,11 @@
+ *   joined. Using expressions in the query may result in column aliases in JOIN

Should be "...column aliases in the JOIN clause..."

Committed 1b87d95 and pushed to 8.6.x. Thanks!

Leaving to port to 8.5.x

Committed f073033 and pushed to 8.5.x. Thanks!