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

Support MySQL datatype SET #330

Open
mikehaertl opened this issue Jun 11, 2014 · 18 comments
Open

Support MySQL datatype SET #330

mikehaertl opened this issue Jun 11, 2014 · 18 comments

Comments

@mikehaertl
Copy link

mikehaertl commented Jun 11, 2014

A MySQL SET datatype should be mapped to an array. This would make it much easier to use in forms, e.g. in a checkboxList(). For now as a developer I have to write getter/setter which do the conversion between a csv string and an array.

public function getUserCountries()
{
    return explode(',', $this->user_countries);
}
public function setUserCountries($value)
{
    $this->user_countries = is_array($value) ? implode(',', $value) : '';
}

Funding

  • You can sponsor this specific effort via a Polar.sh pledge below
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@samdark
Copy link
Member

samdark commented Jun 11, 2014

Set to 2.1 unless someone will work on a pull request. Overall it sounds good.

@mikehaertl
Copy link
Author

I was having a quick look, on how to best implement this. It seems like the conversion DB -> PHP could be done in yii\db\ColumnSchema::typecast(), but this class is not MySQL specific. I don't know about other DBMS but maybe we should add a yii\db\Schema::TYPE_SET type?

I'm just not sure, where to do the backward conversion.

@klevron
Copy link
Contributor

klevron commented Jun 13, 2014

This could be easily done with data converter behavior, I wrote this kind of functionnality for Yii 1, it is really handy. There is a good sample of code about this by @mdmunir here : yiisoft/yii2#3637

yiisoft/yii2#2484

@mikehaertl
Copy link
Author

@klevron I'd prefer to have this done automatically in the core, without having to attach a behavior.

@klevron
Copy link
Contributor

klevron commented Jun 13, 2014

If it is included in core, it should not be done automatically by default.

@mikehaertl
Copy link
Author

Why not? Can you show me a case where you would not want to have a SET being converted to an array? It's the natural representation of a SET in PHP.

@klevron
Copy link
Contributor

klevron commented Jun 13, 2014

Simply because this should be a choice from the dev. When you select a SET column in SQL, you will get a string.

@klimov-paul
Copy link
Member

MySQL SET data type is non-standard SQL feature. Its usage is actually against the Relational Therory and relational approach.
Also it seems logical we return data same way (or at least as close as possible) as PDO does.

@nineinchnick
Copy link
Contributor

Converting a string obtained in the db driver to a PHP array is risky, the implementation is error-prone. That's the reason I avoid using ARRAY data types in PostgreSQL. I'd much more prefer using serialized JSON stored as text.

@mikehaertl
Copy link
Author

MySQL SET data type is non-standard SQL feature. Its usage is actually against the Relational Therory and relational approach.

Still it's a valid data type in MySQL. I can hardly think of any situation, when you would not want to tread this result as an array (remove entries, add entries). All this becomes more cumbersome if it's not an array.

@mikehaertl
Copy link
Author

Converting a string obtained in the db driver to a PHP array is risky, the implementation is error-prone.

I can not agree to that. The string from a SET is very well defined i think. So conversion to/from an Array is no problem at all.

@nineinchnick
Copy link
Contributor

The problem is can you implement a deserializer correctly, I mean escaping separators etc, so all edge-cases are handled.

@mikehaertl
Copy link
Author

The problem is can you implement a deserializer correctly, I mean escaping separators etc, so all edge-cases are handled.

If you couldn't then the SET datatype would be broken by design, no?

@mikehaertl
Copy link
Author

From here:

A SET is a string object that can have zero or more values, each of which must be chosen from a list of permitted values specified when the table is created. SET column values that consist of multiple set members are specified with members separated by commas (“,”). A consequence of this is that SET member values should not themselves contain commas.

@nineinchnick
Copy link
Contributor

I mean the implementation cannot be trivial, that is you can't just do an explode(',',$sql_value). I don't write my own deserializers because I use known, cross-platform formats, so even if I wrote one I probably wouldn't trust it much or spend a large amount of time researching how to do it correctly. I much rather use a different db design or a different format stored in a text field.

I'm not opposing the whole issue, I think it would be a nice addition. I'm just saying the cost of doing this is pretty high and the priority is low and there are other solutions.

@nineinchnick
Copy link
Contributor

Ah, should not or cannot contain comma?. So the usage is quite limited?

@mikehaertl
Copy link
Author

Ah, should not or cannot contain comma?. So the usage is quite limited?

Yeah, but this is is a general limitation of this datatype. Unrelated to the question, whether we do auto-conversion or not (read: if someone uses a comma, he'll run into problems anyway).

@nineinchnick
Copy link
Contributor

Another argument, accepting this creates a precedence where values returned from db sometimes would not be scalar values only. This would require adding extra conditions. On the other hand, when a developer uses a SET in his database he knows where the value is used and can handle conversion himself.

@samdark samdark transferred this issue from yiisoft/yii2 Nov 3, 2018
@Tigrov Tigrov transferred this issue from yiisoft/db Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants