Skip to content

Conversation

@morozov
Copy link
Member

@morozov morozov commented Oct 29, 2025

Fixes #7199.

This is a regression caused by #5089 which removed the MySQL57Platform together with the overridden method. I don't remember why in #4397 I overrode the method only in MySQL57Platform but not in AbstractMySQLPlatform (it seems it would have been the right thing to do).

The regression was possible since at that time we didn't have platform-specific integration tests for the SQL parser. Now we have a couple of them.

@morozov morozov added this to the 4.3.5 milestone Oct 29, 2025
@morozov
Copy link
Member Author

morozov commented Oct 29, 2025

@derrabus could you take a look at the PHPStan failure? It fails on 4.3.x as well and seems related to #7187. PHPStan doesn't fail on 4.4.x and above since 4.3.x hasn't been merged up yet.

 ------ ---------------------------------------------------------------------------------------------------
  Line   tests/Logging/MiddlewareTest.php
 ------ ---------------------------------------------------------------------------------------------------
  40     Parameter #1 $record of method Psr\Log\Test\TestLogger::hasInfo() expects array{level: string,
         message: string|Stringable, context: array{exception?: Throwable}}|string, array{message:
         'Connecting with…', context: array{params: array{user: 'admin', password: '<redacted>'}}} given.
         🪪  argument.type
         💡  Type #1 from the union: Array does not have offset 'level'.

At a glance, the test is correct and PHPStan seems to be right, while the TestLogger method signatures are questionable. For example:

/**
 * @psalm-type log_record_array array{level: string, message: string|\Stringable, context: array{exception?: \Throwable}}
 */

    /**
     * @param log_record_array|string $record
     */
    public function hasInfo($record): bool
    {
        return $this->hasRecord($record, 'info');
    }

The method expects the $record array to contain a level, even though the level is hard-coded in the method implementation. Not passing the level as part of the record seems correct.

@derrabus
Copy link
Member

derrabus commented Oct 29, 2025

the TestLogger method signatures are questionable.

I agree and I stubled accross a similar issue on the new typing already: php-fig/log-test#17

@morozov morozov merged commit d437442 into doctrine:4.3.x Oct 29, 2025
95 of 96 checks passed
@morozov morozov deleted the issues/7199 branch October 29, 2025 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL Parser is initialized with mySQLStringEscaping: false on MySQL platforms

3 participants