This happens with feeds, feed items and custom blocks, couldnt reproduce it with nodes

Attach an entity reference field that references an entity without items..eg tags vocabulary and make sure there are no available terms
run this code

<?php
entity_create
('custom_block', array('info' => 'test', 'type' => 'basic'))->save();
?>

This leads to:

Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'field_test_target_id' cannot be null: INSERT INTO {field_data_field_test} (entity_type, entity_id, revision_id, bundle, delta, langcode, field_test_target_id, field_test_revision_id) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7); Array ( [:db_insert_placeholder_0] => custom_block [:db_insert_placeholder_1] => 2 [:db_insert_placeholder_2] => 2 [:db_insert_placeholder_3] => basic [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => [:db_insert_placeholder_7] => ) in Drupal\Core\Entity\DatabaseStorageControllerNG->save() (line 342 of /var/www/d8/core/lib/Drupal/Core/Entity/DatabaseStorageControllerNG.php).

This issue blocks:

#15266: Replace aggregator category system with taxonomy: (normal task)
#1777956: Provide a way to define default values for entity fields: (normal task)
#1818570: Convert users to the new Entity Field API: (major task)
#1498674: Refactor node properties to multilingual: (critical task)

Files: 
CommentFileSizeAuthor
#31 clean-values-1957888-31.patch7.78 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 55,679 pass(es).
[ View ]
#31 clean-values-1957888-31-interdiff.txt1.37 KBBerdir
#29 clean-values-1957888-29.patch6.41 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 56,496 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#27 clean-values-1957888-27.patch6.41 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch clean-values-1957888-27.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#27 clean-values-1957888-27-interdiff.txt1.66 KBBerdir
#22 drupal-empty_values_exception-1957888-22.patch3.76 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,870 pass(es), 0 fail(s), and 8 exception(s).
[ View ]
#22 interdiff.txt1.76 KBParisLiakos
#19 drupal-empty_values_exception-1957888-19.patch4.2 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 55,421 pass(es).
[ View ]
#17 drupal-empty_values_exception-1957888-17.patch3.64 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,356 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#13 drupal-empty_values_exception-1957888-13.patch2.75 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 54,413 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#10 drupal-empty_values_exception-1957888-10.patch2.69 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 53,892 pass(es).
[ View ]
#8 drupal-empty_values_exception-1957888-8--fail.patch1.65 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 53,946 pass(es), 0 fail(s), and 9 exception(s).
[ View ]
#8 drupal-empty_values_exception-1957888-8.patch2.13 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 53,896 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#2 fail.patch942 bytesParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 53,847 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Comments

There is an array(0 => NULL) in field_sql_storage_field_storage_write as field's item:/

Title:Exception when an entity reference field is empty on pragrammatic creationException when an entity reference field is empty on programmatic creation
Status:Active» Needs review
StatusFileSize
new942 bytes
FAILED: [[SimpleTest]]: [MySQL] 53,847 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

i am lost in the typed data hell:/
here is a test that proves the failure

Status:Needs review» Needs work

The last submitted patch, fail.patch, failed testing.

Issue tags:+Entity Field API

I've been fighting with this one before, I'll try to point @fago or @das-peter to it :)

i am not sure if this is entity_reference's fault, but i am not able yet to reproduce it, with an instance that has no er field attached

Component:entity_reference.module» field system

Delete entity reference field..
Add an image field
Try again

Notice: Trying to get property of non-object in Drupal\file\FileUsage\DatabaseFileUsageBackend->add() (line 54 of core/modules/file/lib/Drupal/file/FileUsage/DatabaseFileUsageBackend.php).
Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'fid' cannot be null: INSERT INTO {file_usage} (fid, module, type, id, count) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => file [:db_insert_placeholder_2] => custom_block [:db_insert_placeholder_3] => 13 [:db_insert_placeholder_4] => 1 ) in Drupal\Core\Entity\DatabaseStorageControllerNG->save() (line 342 of /var/www/d8/core/lib/Drupal/Core/Entity/DatabaseStorageControllerNG.php).

Title:Exception when an entity reference field is empty on programmatic creationException when a field is empty on programmatic creation

Status:Needs work» Needs review
StatusFileSize
new2.13 KB
FAILED: [[SimpleTest]]: [MySQL] 53,896 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
new1.65 KB
FAILED: [[SimpleTest]]: [MySQL] 53,946 pass(es), 0 fail(s), and 9 exception(s).
[ View ]

i managed to track this down to \Drupal\Core\Entity\Field\Type\Field::getValue()
see patch

Status:Needs review» Needs work

The last submitted patch, drupal-empty_values_exception-1957888-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.69 KB
PASSED: [[SimpleTest]]: [MySQL] 53,892 pass(es).
[ View ]

seems this NULL is needed, but we should remove those values before sending to storage...is updateOriginalValues() the correct place?

We struggled several times with this NULL value. However, I don't think we can use array_filter() because this would remove 0 => 0, 0 => FALSE as well.
I try to get fago on board here as I'm kind of lost atm.

indeed..a field can always have a falsey value..
so maybe loop and only remove values that === NULL?

Assigned:Unassigned» fago
StatusFileSize
new2.75 KB
FAILED: [[SimpleTest]]: [MySQL] 54,413 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

something like this..cant think of anything else
i guess waiting for fago here so assigning to him, in case he can take a look

Status:Needs review» Needs work
Issue tags:-Entity Field API

The last submitted patch, drupal-empty_values_exception-1957888-13.patch, failed testing.

Status:Needs work» Needs review

Random failure

#13: drupal-empty_values_exception-1957888-13.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Entity Field API

The last submitted patch, drupal-empty_values_exception-1957888-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.64 KB
FAILED: [[SimpleTest]]: [MySQL] 55,356 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

i copied berdir's fix over from #1818570-38: Convert users to the new Entity Field API :)

Status:Needs review» Needs work

The last submitted patch, drupal-empty_values_exception-1957888-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.2 KB
PASSED: [[SimpleTest]]: [MySQL] 55,421 pass(es).
[ View ]

updated copy-paste to pass tests;)
interdiff is here
http://drupal.org/files/user-ng-interface-1818570-40-interdiff.txt

Hm, shouldn't this rely on the isEmpty() method provided by ComplexDataInterface ?

On the Field API side, we typically run _field_filter_items() to filter out empty field values before they get saved. In #1969728: Implement Field API "field types" as TypedData Plugins I'm precisely moving that behavior (and the underlying hook_field_is_empty() callback implemented by field types) to run on top of isEmpty().

Status:Needs review» Needs work

Hm, shouldn't this rely on the isEmpty() method provided by ComplexDataInterface ?

Exactly, however when you look at Field::getValue() that is already what is happening there. I think having a NULL value in there is perfectly fine, but I'd see it as the job of the storage to not store them.

Else, we could remove the NULL values in Field::getValue() but I do not think that's reasonable, e.g. if you have

<?php
$list
= array(
$item1,
$items2,
$item3
);
?>

and you set $list[1] to something empty, we cannot have magically re-number the array. I think this re-numbering should happen with the store/load operation, i.e. skip storing NULL/empty values and so skip loading them the next time.

Status:Needs work» Needs review
StatusFileSize
new1.76 KB
new3.76 KB
FAILED: [[SimpleTest]]: [MySQL] 55,870 pass(es), 0 fail(s), and 8 exception(s).
[ View ]

something like this?

Status:Needs review» Needs work

The last submitted patch, drupal-empty_values_exception-1957888-22.patch, failed testing.

I ran over this problem with #1777956: Provide a way to define default values for entity fields also and fixed it there as it was necessary to fix a test fail.

The patch there adds a method cleanValue() which cleans up the field value, i.e. removes empty items and re-keys the array. While I still think that field storage should just skip saving empty items, I think there is value in having a separate method for cleaning up anyway - consider you want to render a preview of a user-submitted form data. Cleaning-up should probably happen after extracting form values, so having it as a separate method makes sense to me.

Cleaning-up should probably happen after extracting form values, so having it as a separate method makes sense to me.

That's how field API did it so far: WidgetBase::extractFormValues() calls _field_filter_items().
In my ongoing work on #1969728: Implement Field API "field types" as TypedData Plugins I switched that part over to calling a filterEmpty() method I added on the Field class, and which turns out to be pretty similar to the cleanValue() method you add in #1777956: Provide a way to define default values for entity fields :-).

Priority:Major» Critical

Escalating this to critical as suggested by @Berdir since this is blocking at least:

#1777956: Provide a way to define default values for entity fields (normal task)
#1818570: Convert users to the new Entity Field API (major task)
#1498674: Refactor node properties to multilingual (critical task)

Issue summary:View changes

Fixed typo

Status:Needs work» Needs review
StatusFileSize
new1.66 KB
new6.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch clean-values-1957888-27.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Let's see if I got this right, extracted fago's fix and added the tests from this issue.

Status:Needs review» Needs work

The last submitted patch, clean-values-1957888-27.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.41 KB
FAILED: [[SimpleTest]]: [MySQL] 56,496 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Huh?

Status:Needs review» Needs work

The last submitted patch, clean-values-1957888-29.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.37 KB
new7.78 KB
PASSED: [[SimpleTest]]: [MySQL] 55,679 pass(es).
[ View ]

Ah, overlooked that change.

+++ b/core/modules/forum/forum.module
@@ -334,10 +334,13 @@ function forum_node_presave(EntityInterface $node) {
+      // Only do a shadow copy check if this is not a new node.

Shouldn't that be "Only do a shadow copy if this is not a new node."?
The word "check" seems to be a leftover from an earlier sentence.

Found nothing else to nag about. :)

check makes sense IMHO. We only check if we need to create a shadow copy if the node is not new. We don't always create one, we just check if we have to.

Status:Needs review» Reviewed & tested by the community

Well, RTBC then I guess :)
Berdir asked me via IRC to mention here that fago should be added to the authors since the patch contains ode created by him.

Note: We also have more specific test coverage for this in #1822000: Remove Drupal\field_test\Plugin\Entity\Type\TestEntity in favor of EntityTest as well (FieldAttachOtherTest::testFieldAttachExtractFormValues()) which is failing as well with EntityNG when porting from TestEntity to EntityTest.

Status:Reviewed & tested by the community» Fixed

Committed 530a6ed and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

issues blocked by this