-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
PDO : add method getColumnMeta #6109
Conversation
Description : So we need to recover the method getColumnMeta as we had it in the version 2. This merge may handle the same problem that the ticket #3534. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm against this change in its current form for multiple reasons:
- The description does not explain why this change has been made.
- The abstraction is leaky: We expose a data structure specific to PDO that other drivers would have to emulate.
- Implementations for non-PDO drivers are missing.
- Not a single test has been written for this feature.
- The
final
keyword has been removed from multiple classes and various internal properties were switched toprotected
without any need.
/** | ||
* Returns the column information. | ||
*/ | ||
public function getColumnMeta($index): array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change: We cannot add new methods to existing interfaces without breaking userland implementations.
Thank you for your comments @derrabus. You were right , the final attribute and the protected have not to be removed, it was because of previous tests. The need we have is to recover the metadata of a column in a query. Before the version 3 , we was using the getColumnMeta method, but it was removed in the version 3. So we need to recover the method getColumnMeta as we had it in the version 2. |
I will write tests if the idea is approved. |
Thank you for the heads up. I'm afraid, "I'm adding the method because it had been removed" is not really enough. The method had been removed because we stopped extending PDO. Simply undoing this change would reintroduce the leaky abstraction that we've just gotten rid of.
That issue is about exposing the native result object so that functionality can be accessed that we don't provide an abstraction for. Accessing column metadata was only one motivation behind that, so your PR would not entirely solve that issue. |
Thank you for your answers. For more details, wa are using symfony with doctrine, and we were not using object in all our application. Sometimes, when the queries needed a huge quantity of data, we use mysql queries without using objects. |
PHP does not have an equivalent of the decimal type, so strings are fine here. Note that casting decimals to float may cause a loss in precision due to rounding errors. If a float is actually what you want, you should use a float or double in MySQL instead. And PDO_mysql should return MySQL integers and floats as PHP integers and floats nowadays. I think, this has been improved in PHP 8.0.
Why do you need that kind of guesswork? If I send a query to a database server, e.g. SELECT my_int_column FROM my_table; Why do I need any guesswork to determine what columns I get and which types they have? My example may be trivial, but what I've said applies to even the most complex queries: I as the author of the query know that I've asked the server for, so nothing that the server returns should be a surprise. The only exception I can think of if that the SQL itself is user input.
Okay, so it's only about the type information. I believe it would be a great improvement if our results would expose information on their columns and types in a documented and portable way so that we can implement it across all drivers. But that's more than just dumping the PDO metadata. So, my question is: After everything that I told you, do you still believe that you need type information on your results? And if so, do you want to work on a concept and implementation for an abstraction layer that works on all drivers and not just |
I would like it working without any conversion. |
It is possible without "getting the types" because you as the author of the SQL query should know exactly which columns that query will yield. |
Ok, so for every query, i have to put the type i want. |
Pretty much. It's your database, it's your query. You should be able to tell what the type of the result column |
The problem is that we have to do this for every query. |
All right. Shall we close this PR then? |
Currently, we can. |
Summary