Prevents incorrect table aliases

[gquemener]

When one uses a discriminator map with a valid, but incorrectly cased,
classname (Foo\Bar\baz instead of Foo\Bar\Baz for instance), the
class metadata won't complain (php being case insensitive when it comes
to class name). However, it will complain if the class does not exist.

This will lead to "exotic" table alias being generated when querying the
parent class because the sqlTableAliases won't contain the wrongly typed
classname.

Here's an error example:

An exception occurred while executing 'SELECT t0.a AS a_3, t0.b AS b_4, t0.c AS c_5,
t0.id AS id_6, t0.d_id AS d_7, t0.e AS e_8, t0.discr, t1.a AS a_9, t1.b AS b_10, 
t1.c AS c_11, t1.d AS d_12, t1.e AS e_13, t1.f AS f_14, t16.a AS a_15 FROM table1 t0
LEFT JOIN table2 t1 ON t0.id = t1.id LEFT JOIN table3 t2 ON t0.id = t2.id':

SQLSTATE[42S22]: Column not found: 1054 Unknown column 't16.a' in 'field list'

t16 should be t2

This is a quite hard to debug issue as the typo is most of the time quite hard to spot and the error (triggered by the db itself) doesn't make you think about a mapping issue at first sight.

I don't think this is an issue in the metadata loader, as class_exists is case insensitive by design, so should probably be this internal map. EDIT: I have switched side on this question

I've scratched my head for a while to know where I could add a test case to show you exactly the issue, but I couldn't. Please point me to the right direction if you think this fix should make it to master.

Thanks 👋

[gquemener]

This is a map from FQCN to table alias, I'm not sure why $tableName is not named $fqcn 🤔

[gquemener]

Related to #8122

[greg0ire]

Do you really mean to target master?

[gquemener]

One might consider this as a BC break, don't you think? 🤔

However, a quick search shows that this CachedPersisterContext::$sqlTableAliases is only read/written from the BasicEntityPersister. So this change could be considered BC (as it only touches at internal object communication). If so, I could change the target branch to 2.7.

Nonetheless, please note that the underlying problem could also be fixed by merging #8122 (which prevent case typo in the discriminator map to happen in the first place).

Both PRs could be merged as well, I guess 🤔

I am quite opened to suggestion here, as long as this or (non exclusive) the other PR makes it to 2.7/master :)

[gquemener]

BTW, do you know what's going on with the CI? It seems broken on master as well

[greg0ire]

BTW, do you know what's going on with the CI?

I'll ask internally but I think I already did some time ago 😅

[SenseException]

I would consider this a BC break if the changes are breaking any tests. If CachedPersisterContext would get accessor methods for $sqlTableAliases that handles the case sensitivity, it would be something that could fit to the master branch. At this point every access to $sqlTableAliases from anywhere else, that doesn't use the lower case, would be open for bugs.

BTW: Tests would be nice.

[gquemener]

I agree.

However, after checking in the codebase, the CachedPersisterContext is only used in one place (and potentially in any other library out there, though the risk is really limited given the purpose of this class).

I believe the property is public for performance purpose. If so, introducing a mutator (and reducing visibility of the prop) is not really possible.

Furthermore, the initial issue can be fixed by #8122.

Consequently, I would suggest to close this PR.

[beberlei]

Fixed by #8122