Skip to content

Drop pdo_mysql_prepare_load_data.phpt #6509

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

Closed

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 11, 2020

Like the test title and some comments in this test describe, this test
was supposed to have ::prepare() failing because LOAD DATA INFILE
would not be supported as prepared statement, and then the test checks
whether follow-up queries would succeed. However, LOAD DATA INFILE
is supported for prepared statements at least on Windows with mysqlnd,
so the test does no longer test what it is supposed to do. Therefore,
we drop it.


@nikic, @kamil-tekiela, do you see a way to salvage the original test intend by using some other queries which are not supported by prepared statements?

Like the test title and some comments in this test describe, this test
was supposed to have `::prepare()` failing because `LOAD DATA INFILE`
would not be supported as prepared statement, and then the test checks
whether follow-up queries would succeed.  However, `LOAD DATA INFILE`
is supported for prepared statements at least on Windows with mysqlnd,
so the test does no longer test what it is supposed to do.  Therefore,
we drop it.
@kamil-tekiela
Copy link
Member

Why remove it completely? If we know that it works now, then we can keep it. We could merge it with pdo_mysql_exec_load_data.phpt but I don't really see a point in removing it.
We should improve it as at the moment (even with my fixes yesterday) the test is not written very well. I can see if I can improve it more, I only did what was necessary to get it working again. I am not sure about Linux, but the test should be working on both Windows and Linux without any problems.

@nikic
Copy link
Member

nikic commented Dec 11, 2020

The relevant behavior is already tested in https://github.com/php/php-src/blob/master/ext/pdo_mysql/tests/bug70066.phpt using USE, which is also not supported in the PS protocol. As such, I agree that it makes more sense to drop this test entirely.

@php-pulls php-pulls closed this in 10c9d61 Dec 14, 2020
@cmb69 cmb69 deleted the cmb/pdo_mysql_prepare_load_data.phpt branch December 14, 2020 17:37
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.

3 participants