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

CMysqlDbColumnSchema::typecast() should override default implementation to tune typecasting per DBMS #92

Closed
qiangxue opened this issue Feb 15, 2012 · 3 comments

Comments

@qiangxue
Copy link
Member

For DB type DECIMAL the PHP type is set to 'string'. I wonder if 'float' wouldn't make more sense to also guarantee correct handling of empty/NULL values. (BTW again i'd suggest to replace 'double' with 'float' in extractType() for consistency'.)

If you assign '' to an AR attribute of DB type 'DECIMAL DEFAULT NULL' it will save 0.00 to DB instead, because CMysqlDbColumnSchema::typecast() only converts '' to null, if 'gettype($value)!==$this->type', which is not true for DECIMAL due to the above ('string'==='string'). If type where 'float' it would trigger the type cast.

Besides that i wonder what line 29 is for:

else if(strpos($dbType,'float')!==false || strpos($dbType,'double')!==false)

AFAIK neither 'float' nor 'double' are valid MySQL datatypes.

Also not sure about detection of 'boolean'. Shouldn't that be 'tinyint(1)' instead?

Migrated from http://code.google.com/p/yii/issues/detail?id=1750


earlier comments

qiang.xue said, at 2010-11-10T16:04:20.000Z:

Using float to represent decimal may lose precision (see the precision range of decimal http://dev.mysql.com/doc/refman/5.1/en/precision-math-decimal-changes.html)

For decimal type, you will need to convert empty strings into nulls. (you can write a validator to do this conversion).

Double is a valid MySQL data type.

We used to treat tinyint(1) as boolean. There was an issue reported about it. So we changed it back to integer.

haertl.mike said, at 2010-11-10T18:44:13.000Z:

I see, thanks for clarification. I've already seen the extension that does '' to 'null' conversion as validator but find it quite obscure that some types are auto converted, others not. It's not documented either.

Couldn't CDbMysqlColumnSchema override typecast() and also check dbType (+ maybe allowNull) there, and also auto convert empty DECIMALs to null?

This could be done for every DBMS to fine tune type casting.

qiang.xue said, at 2010-11-10T20:08:18.000Z:

This is a good idea. Other than typecasting '' to null for specific db types, what else do we need to handle?

haertl.mike said, at 2010-11-12T12:36:52.000Z:

Changed the issue topic now.

Can't think of anything else. I see that all INT types are already handled: INTEGERs have (at least) the same range in PHP like in MySQL and BIGINTs become strings in PHP. String types should not be a problem neither should BLOBs.

Maybe an option Yii 2.0: Conversion of DATETIME into native DateTime object in PHP (>5.2.0)? Only brainstorming, though.

qiang.xue said, at 2010-11-12T13:24:08.000Z:

Yes, I think in 2.0, besides php type and db type, we will introduce a new type in between, which includes a richer set of standard types, such as integer, float, datetime, etc.

haertl.mike said, at 2010-11-12T13:45:01.000Z:

Great. So any chance the '' to null conversion for decimals will make it into 1.1.5? I could test it in a project i have.

With respect to your comment #3 i dared to set this ticket to 'Accepted' :)

qiang.xue said, at 2010-11-12T13:48:22.000Z:

We need to handle not just mysql. I'm afraid we are out of time for this to be included in 1.1.5 unless you have bunch of time to burn today so that the fix can be tested for most DBMS.

haertl.mike said, at 2010-11-12T13:52:15.000Z:

Ok, agree that this needs thorough testing. I could only test MySQL here. So 1.1.6 is fine, too.

qiang.xue said, at 2010-11-12T13:54:45.000Z:

Out of time. Set for 1.1.6 milestone.

qiang.xue said, at 2011-01-15T19:25:08.000Z:

Set for next milestone.

qiang.xue said, at 2011-03-26T17:58:16.000Z:

Out of time. Move to next milestone.

michael.imstepf said, at 2011-05-06T11:10:52.000Z:

Not sure how closely related this is to the problem described above but when having an INT column in the database that can be NULL and has the default value of NULL, Yii saves a 0 instead of a NULL if the field is blank.

egarcam said, at 2011-07-22T00:16:27.000Z:

I have the same problems with DECIMAL datatype in MSSQL, Yii try to save an empty string when the field is empty, it must try to save a NULL value. Also happen with VARCHAR datatype, Yii save a blank, the correct must be a NULL if the field isNullable (can be a new property for each column in the model).

DCaseyTucker said, at 2011-09-19T23:35:42.000Z:

The current work-around is to provide default values for the field and make the field required. I'm not happy about this, but at least I can code around the issue for now.

qiang.xue said, at 2012-01-01T03:36:53.000Z:

set for 1.1.10 milestone

qiang.xue said, at 2012-01-01T03:37:09.000Z:

set for 1.1.10 milestone

qiang.xue said, at 2012-01-01T03:37:35.000Z:

set for 1.1.10 milestone

@mikehaertl
Copy link

I think the label for this issue is wrong. It affects all DBMS, not only MSSQL.

@teian
Copy link

teian commented Mar 14, 2013

Is there any update on this one?

@matih
Copy link

matih commented May 16, 2013

I ask also, any updates??

@samdark samdark closed this as completed Nov 17, 2015
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