TYPO3 Core

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61880

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61880

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61880

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61880

The related task has been merged for version 10 and is available in 10.4.

Futhermore, there is a "workaround" that works already with version 9, that is described in a PR for the official documentation https://github.com/TYPO3-Documentation/TYPO3CMS-Reference-CoreApi/pull/840

There is a new extension cc_dbal_leftjoin which fixes this behavior. Just install it and add true as additional parameter for leftJoin.
https://extensions.typo3.org/extension/cc_dbal_leftjoin/

//$queryBuilder->leftJoin($fromAlias, $join, $alias, $condition);
$queryBuilder->leftJoin($fromAlias, $join, $alias, $condition, true);

So SQL changes:

# SELECT * FROM table1 LEFT JOIN table2 ON table1.uid = table2.field WHERE table1.deleted = 0 AND table2.deleted = 0;
SELECT * FROM table1 LEFT JOIN table2 ON table1.uid = table2.field AND table2.deleted = 0 WHERE table1.deleted = 0

Markus Klein wrote:

Sorry, not solved. Was only a feature for v10 with #87776

Have you seen the "workaround" described in the documentation (still in review on Github https://github.com/TYPO3-Documentation/TYPO3CMS-Reference-CoreApi/pull/840).

Is there something missing?

I don't want workarounds if there is a proper solution. I'm expecting the database API to work as intended.

Markus Klein wrote:

I don't want workarounds if there is a proper solution. I'm expecting the database API to work as intended.

If #87776 solves the issue at hand, I don't understand why new API is introduced that does the same.
Besides that, I'm not sure whether the framework API can understand the intention of the user.
Instead the API should allow to build an SQL query that reflects the intention. As far as I can see,
this can be done with #87776

My use case and why I think this must be fixed in Core (the default behaviour)

Table User

--------------
| uid | name |
--------------
|   1 | Max  |
|   2 | Sue  |
|   3 | Luke |
--------------

Table Speaker

-----------------------------------
| uid | userid | rating | deleted |
-----------------------------------
|  10 |      1 |      5 |       0 |
|  11 |      2 |      4 |       1 |
-----------------------------------

$qb->select('u.*', 's.rating')
   ->from('user', 'u')
   ->leftJoin('u', speaker, 's', 's.userid = u.uid')
   ->orderBy('u.uid');

Expected result:

-----------------------
| uid | name | rating |
-----------------------
|   1 | Max  |      5 |
|   2 | Sue  |   null |
|   3 | Luke |   null |
-----------------------

Actual result:

-----------------------
| uid | name | rating |
-----------------------
|   1 | Max  |      5 |
-----------------------

"Side note": The worst thing about the leftJoin() is that if the joined table has NO enableFields, it behaves like expected!

The worst thing is that this even needs discussion.

The API provides methods for outer joins. Commanding the API to do an outer join must do an outer join and no inner join. Currently it does an implicit inner join. Which is a bug. Period.

Any outer join, left or right, must either
1) not have any implicit conditions on the joined tables (only the ones explicitly requested by the calling code)
2) or having OR joined_table.field IS NULL added to all implicit field conditions

Clean code requires that the code does what it claims it's doing. If this can not be satisfied, drop the API at all to save everyone's time.

Sorry for the upcoming rant, but the core development more and more takes the direction into a state where I could post this rant under a lot of tickets, I have the impression.

The existance of a workaround does not free from being in the responsibility to provide a proper bugfix. Or else you could close down forge entirely, because there's a workaround for every bug ("just write your own patch").

As far as I'm concerned TYPO3 9 is still a LTS version and it's still in its support period and claims to be an enterprise CMS. And companies/people that are no core devs themselves still contribute with membership fees, by providing bug reports, spreading the word about TYPO3, helping others in Slack or even doing code submissions. So they can expect that bugs are treated as bugs and get fixed. Do all these words still mean anything these days?

Stefan P wrote:

The worst thing is that this even needs discussion.

The API provides methods for outer joins. Commanding the API to do an outer join must do an outer join and no inner join. Currently it does an implicit inner join. Which is a bug. Period.

Any outer join, left or right, must either
1) not have any implicit conditions on the joined tables (only the ones explicitly requested by the calling code)
2) or having OR joined_table.field IS NULL added to all implicit field conditions

Clean code requires that the code does what it claims it's doing. If this can not be satisfied, drop the API at all to save everyone's time.

Sorry for the upcoming rant, but the core development more and more takes the direction into a state where I could post this rant under a lot of tickets, I have the impression.

The existance of a workaround does not free from being in the responsibility to provide a proper bugfix. Or else you could close down forge entirely, because there's a workaround for every bug ("just write your own patch").

As far as I'm concerned TYPO3 9 is still a LTS version and it's still in its support period and claims to be an enterprise CMS. And companies/people that are no core devs themselves still contribute with membership fees, by providing bug reports, spreading the word about TYPO3, helping others in Slack or even doing code submissions. So they can expect that bugs are treated as bugs and get fixed. Do all these words still mean anything these days?

I am all with you!

Just two notes from my side:

1) Discussion is IMO always a good thing to prevent wrong decisions and to maybe find a better solution one didn't think of. But discussions must end at some point.
2) Core development - specifically bug fixing - can easily be described as "fear driven development". Fixing a bug, specifically one that was experienced by many, always causes problems for those having a workaround in place already, which the fix then breaks. I can tell you from my own years-long experience that no matter how unlikely it seems that some misbehaviour has caused a problem for someone, a few days after the release of the fix the first complaint rushes in on forge. That's kind of a natural law. So, we have been extremely careful in the past on what to fix, in which version and how.

Besides this carefulness, I strongly agree that this very problem MUST be fixed in all supported versions!

Fixing a bug, specifically one that was experienced by many, always causes problems for those having a workaround in place already, which the fix then breaks.

The way must always be: fix it to match the intention. Not fixing it with the reason "there are people smart enough to workaround" makes it just worse (especially for those not smart enough to workaround, but smart enough to trust the API). Any other way would introduce an ever growing "climate of carelessness" over time: if a discovered bug has no "consequences" (because one can "workaround" it) people fall into a state of not caring at all about feature being implemented correctly in the first place. This is not good for a development community (this even has nothing to do with paid or free or FOSS or closed source, but with a general attitude of how people think about code quality).

People who are able to workaround, are also able to drop their workaround soon enough.

Same issue here. leftJoin is simply broken when using it with tables having restrictions. The way EXT:cc_dbal_leftjoin solves it is the correct one in my opinion, those conditions must be added to the join condition.

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64937

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64937

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64937

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/64937

Patch set 1 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68740

Patch set 2 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68792

Patch set 3 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68792

Patch set 4 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68792

Patch set 5 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68792