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

SqLite tries to insert NULL values into NOT NULL fields. #4715

Closed
dereuromark opened this issue Sep 24, 2014 · 10 comments
Closed

SqLite tries to insert NULL values into NOT NULL fields. #4715

dereuromark opened this issue Sep 24, 2014 · 10 comments
Milestone

Comments

@dereuromark
Copy link
Member

When saving just city field this occurs:

PDOException: SQLSTATE[23000]: 
Integrity constraint violation: 19 addresses.first_name may not be NULL

As you can see from https://travis-ci.org/dereuromark/cakephp-geo/builds/36043030 the tests for Mysql pass just fine.
Also, when debugging the input prior to saving it is clear that non of the fields in question are marked as dirty or are touched in any way. The sql going into the Connection class looks fine, as well.

So for some reason SqLite takes all schema fields that are not part of the created record and tries to insert them as NULL - which will fail for all NOT NULL fields, of course.
Maybe it should check the schema and apply an empty string where necessary instead.

@lorenzo
Copy link
Member

lorenzo commented Sep 24, 2014

Please include the generated SQL for the failing queries.

@thaJeztah
Copy link

Maybe it should check the schema and apply an empty string where necessary instead.

I'm -1 on this. I suspect MySQL is passing the tests because it silently ignores not-null constraints and automatically substitutes them with empty strings (or, for a date field with 0000-00-00, which is even worse). I think that this test also fails on PostgreSQL and SQL Server?

Basically, MySQL is wrong here; after all, marking a field as NOT NULL means it is a required field, so trying to insert a record without that field set, must fail to guarantee data-integrity.

CakePHP could instead automatically generate and perform validation-rules based on the schema and invalidate the record before attempting to save it to the database.

@thaJeztah
Copy link

For additional reading on MySQL behaving badly, it is possible to make MySQL more compliant with the standards by setting the Server SQL Mode. Documentation in this can be found here; http://dev.mysql.com/doc/refman/5.7/en/sql-mode.html

@markstory
Copy link
Member

MySQL is the worst. You should get the SQL you write and deal with the consequences. I suspect that both postgres and sqlerver would fail in similar ways.

@dereuromark
Copy link
Member Author

The queries run are:

'INSERT INTO "addresses" ("city") VALUES (:c0)'
'INSERT INTO "addresses" ("city", "lat", "lng", "formatted_address") VALUES (:c0, :c1, :c2, :c3)'

and debugging the Connection object:

\vendor\cakephp\cakephp\src\Database\Connection.php (line 255)
########## DEBUG ##########
object(Cake\Database\Statement\SqliteStatement) {
        [protected] _count => (int) 0
        [protected] _records => []
        [protected] _allFetched => false
        [protected] _counter => (int) 0
        [protected] _statement => object(Cake\Database\Statement\PDOStatement) {

                [protected] _statement => object(PDOStatement) {
                        queryString => 'INSERT INTO "addresses" ("city") VALUES
(:c0)'
                }
                [protected] _driver => object(Cake\Database\Driver\Sqlite) {

                        'connected' => true

                }
        }
        [protected] _driver => object(Cake\Database\Driver\Sqlite) {

                'connected' => true

        }
}
###########################

So as you can see I don't even touch that "first_name" field etc.
It should not error in this case but just save as it normally would with other DB engines.

@thaJeztah
Copy link

The error was;

addresses.first_name may not be NULL

The insert query does not contain a value for first_name, so (unless that column has a default value in its schema), it looks like SQLite is doing the right thing here; it refuses to insert a record that is missing required fields.

@lorenzo
Copy link
Member

lorenzo commented Sep 25, 2014

Closing as the queries seem to be doing what they should and are not responsible for inserting null values in the database, which was our suspicion. Please fix your code so it saves all required columns for your schema.

@dereuromark
Copy link
Member Author

Almost correct. But it helped me to track down the problem. It was the upgraded 2.x fixture.
The fixture states (incorrectly):

'null' => false, 'default' => null

Which then breaks SqLite.
Fixing it as

'null' => false, 'default' => ''

seems to resolve the issue.

So I think the ORM should throw an exception here instead when someone tries to use default => null with null => false instead of continuing here cloaking it with a not so clear error message.

@dereuromark dereuromark reopened this Sep 25, 2014
@lorenzo
Copy link
Member

lorenzo commented Sep 25, 2014

so how do you propose a schema where the column needs to be not null and if the column is missing you get the error? It looks to me that 'null' => false, 'default' => null is exactly that

@ADmad
Copy link
Member

ADmad commented Sep 25, 2014

Closing as related PR #4731 is opened.

@ADmad ADmad closed this as completed Sep 25, 2014
ravage84 added a commit to ravage84/cakephp that referenced this issue Feb 16, 2017
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