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

CDbColumnSchema::typecast() issue #265

Closed
qiangxue opened this issue Feb 15, 2012 · 1 comment
Closed

CDbColumnSchema::typecast() issue #265

qiangxue opened this issue Feb 15, 2012 · 1 comment
Labels
Milestone

Comments

@qiangxue
Copy link
Member

What steps will reproduce the problem?

  1. Create a table with a datetime type column in it.
  2. Generate the model with Gii for that table.
  3. Generate the CRUD with Gii for that model.
  4. Create a new model entry in that table from the CRUD with an empty value in that column's corresponding form input.

What is the expected output? What do you see instead?

To be able to save the new record in the database, with that column's value set to null and not an incompatible empty string that will couse an exception to be thrown.

What version of the product are you using? On what operating system?

v.1.1.8

Please provide any additional information below.

I edited the function code to first test if the value is an empty string and if the column allows null values and returned null instead.

public function typecast($value)
{
    if ('' == $value && $this->allowNull) {
        return null;
    }

    if(gettype($value)===$this->type || $value===null || $value instanceof CDbExpression)
        return $value;
    if($value==='')
        return $this->type==='string' ? '' : null;
    switch($this->type)
    {
        case 'string': return (string)$value;
        case 'integer': return (integer)$value;
        case 'boolean': return (boolean)$value;
        case 'double':
        default: return $value;
    }
}

Is there another way to achieve the same result without altering the Yii code ?

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


earlier comments

cebe.cc said, at 2011-12-26T23:34:38.000Z:

You should check on Models beforeSafe() method if the column is an empty string and set it to null if you want null value in database.

alexander.makarow said, at 2011-12-26T23:51:20.000Z:

Proposed solution will prevent saving empty strings.

office@thisway.ro said, at 2011-12-27T08:41:15.000Z:

I don't think this should be checked every time in every model. A better place is here, in the DAL.

alexander.makarow said, at 2011-12-27T13:00:34.000Z:

OK, but what about empty strings?

office@thisway.ro said, at 2011-12-27T19:58:34.000Z:

Why would you want to store in the database an empty string instead of the NULL value ? It's not DB compatible (take for example the datetime data type).

qiang.xue said, at 2011-12-27T20:41:47.000Z:

Empty string and null ARE different (otherwise why does database support NULL?). The former means you have modified the column (even though you supplied an empty string), the latter means you have not touched the column. While the difference seems subtle, it does exist and is often exploited.

It's better to do the empty string to null conversion in upper layers (such as using a validation rule).

office@thisway.ro said, at 2011-12-28T09:39:42.000Z:

You are right, the empty value might be useful in some cases, although the study case you proposed isn't at all a recommended way to write an application :). But this doesn't change the fact that this is a type casting issue and should be addressed in a centralized matter.

If you want to store an empty string in an incompatible database column it should return and store NULL but it should also allow you to store an empty string value in compatible columns only.

The data types compatibility issue is neglected in this typecast implementation.

qiang.xue said, at 2011-12-29T22:23:04.000Z:

Sorry, I misunderstood the problem. I agree we should turn empty string into null for non-string types. Thanks.

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

set for 1.1.10 milestone

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

set for 1.1.10 milestone

@samdark
Copy link
Member

samdark commented Sep 8, 2012

Can be solved using default validator. Closing.

@samdark samdark closed this as completed Sep 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants