TYPO3 Core

But if the field list is empty than no query is generated at all. In this case the functin returns nothing (void) and it's ok that debug_lastBuiltQuery is not set.

The query is not generated in error case? It is bad behaviour to accept the parameters and simply do nothing. It should at least set the $this->store_lastBuiltQuery to the query which would have been created, and show the wrong query. Otherwise it is very hard to find out that nothing has been done in this function. This is needed, because only this is output as sqlDebug. Or invent a second variable for such a case. It is important that a wrong SQL is output in every error case.

The docs says it will return FALSE, in fact it returns nothting. You may check the failure case with code like that:

$query = UPDATEquery(..);
if (is_string($query)) {
// ok
} else {
//error
}

But I think this may fail with DBAL.

Best would be to do what the docs say: return FALSE in case of an error. So you can check it like this:

$query = UPDATEquery(..);
if ($query===FALSE) {
// error
} else {
// ok
}

Yes, this would be fine to do such a check in the program code.
But the sqlDebug should still show this buggy SQL and not another SQL query which is correct.

Why, the funciton did not generate any SQL code. If it does anything with lastGeneartedQuery it should set it to an empty string, but I think this is dull.

IMHO it would be better if all parameter checking in this function (there is one that die's) and the filed list check would throw an exception.

But the function description says:
"Creates an UPDATE SQL-statement for $table where $where-clause (typ. 'uid=...') from the array with field/value pairs $fields_values."
So the caller of this function assumes that this query will be created, even if an error occurs.

If you do not want to use lastGeneratedQuery, then create another one: lastErroneousSQL
This would help the caller of the function to display this for error debugging.

IMHO the check
if (is_array($fields_values) && count($fields_values)) {
is not necessary. It should create the wrong UPDATE query. This function should not avoid this. It is still possible to produce a wrong SQL UPDATE statement. Why does the function check this case?

A die is the worst solution.

I have supplied a patch: martix7015.patch

• Leaving debug_lastBuiltQuery unaltered - which results in very confusing debug output since it would contain a previous unrelated query.
• Setting debug_lastBuiltQuery to the empty string - which would let the debug output show the actual return value, but not provide any information about the cause of the error either.
• Setting debug_lastBuiltQuery to the actual query that was asked for and let the debug output show what caused the error.

The supplied patch uses the last suggestion (even when $fields_values is not an array)

• Now the function always sets debug_lastBuiltQuery to the requested query.
• The return value when an error occurs is no longer NULL but boolean FALSE as stated in the description.

Committed 7015_v2 to trunk, rev. 7708