Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add an UPDATE with ORDER BY that fails. #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rguico
Copy link

@rguico rguico commented Mar 7, 2024

This PR demonstrates a corner case that we use in our code, where UPDATE is used with ORDER BY. I'm not sure how to resolve it, so it's a WIP.

The root cause is that ORDER BY causes the row index to reset in this statement:

return new QueryResult(array_values($rows), $result->columns);

However, removing array_values causes errors in other tests!

@rguico
Copy link
Author

rguico commented Mar 7, 2024

Error caused:

/opt/homebrew/bin/php php-mysql-engine/vendor/phpunit/phpunit/phpunit --configuration php-mysql-engine/phpunit.xml.dist --filter "/(Vimeo\\MysqlEngine\\Tests\\EndToEndTest::testUpdateWithOrderAndPrimaryKey)( .*)?$/" --test-suffix EndToEndTest.php php-mysql-engine/tests --teamcity
Testing started at 10:38 AM ...
Cannot load Xdebug - it was already loaded
PHPUnit 9.6.17 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.1
Configuration: php-mysql-engine/phpunit.xml.dist
Random Seed:   1709829489


Vimeo\MysqlEngine\Processor\SQLFakeUniqueKeyViolation : Duplicate entry '7' for key 'PRIMARY' in table 'video_game_characters'
 php-mysql-engine/src/Processor/Processor.php:226
 php-mysql-engine/src/Processor/UpdateProcessor.php:25
 php-mysql-engine/src/FakePdoStatementTrait.php:237
 php-mysql-engine/src/Php8/FakePdoStatement.php:16
php-mysql-engine/src/FakePdoTrait.php:185
php-mysql-engine/tests/EndToEndTest.php:1144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant