From #1735118: Convert Field API to CMI. Not really sure how to explain this one, so I'll just quote what we said on-issue.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -0,0 +1,463 @@
+      if (array_intersect(array_keys($schema['columns']), field_reserved_columns())) {

@xjm:

It looks like field_reserved_columns() returns only 'deleted'. A procedural wrapper to return that seems odd; followup?

@swentel:

Not for now, this function is also used in _field_sql_storage_columnname() for instance which does not get a $field object

@yched:

Yeah, I actually have no clue what the logic around field_reserved_columns() is. This was introduced by the EFQv2 patch, and the issue has no explanations about that.

But yeah, I tried to see if it could be moved to a method on Field.php, but the consuming code won't allow that.

Files: 
CommentFileSizeAuthor
#9 field-reserved-columns-1969136-9.patch3.45 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 55,723 pass(es).
[ View ]
#9 interdiff.txt2.47 KBswentel
#6 field-reserved-columns-1969136-6.patch3.46 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 55,957 pass(es).
[ View ]

Comments

As far as entity_query() is concerned, ->condition('field_name.deleted', 0) needs to be recognized as a valid condition and this is the way it recognizes it. Field.php refuses creating those columns to avoid confusion.

Right, makes sense. Having a procedural wrapper is not that bad, the function explains what it does and it's better than hardcoding it. So as far as I'm concerned, we can just close this one.

I'd tend to inline / hardcode that "list of forbidden column names" (currently restricted to 1 item...) directly in the Field.php class...
I don't really see the value of delegating this to an external functional helper now ?

@yched static method then ? e.g. _field_sql_storage_columnname does not get a field object ..

Status:Active» Needs review
StatusFileSize
new3.46 KB
PASSED: [[SimpleTest]]: [MySQL] 55,957 pass(es).
[ View ]

Let's move this to a static method.

Status:Needs review» Needs work

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -446,7 +446,7 @@ public function getSchema() {
+      if (array_intersect(array_keys($schema['columns']), $this->field_reserved_columns())) {

I think the recommended practice is static::method().

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/Field.phpundefined
@@ -563,5 +563,13 @@ public function unserialize($serialized) {
+  public static function field_reserved_columns() {

Method name needs to get CamelCased :-)

I think the name of the method should be getReservedColumns().

Status:Needs work» Needs review
StatusFileSize
new2.47 KB
new3.45 KB
PASSED: [[SimpleTest]]: [MySQL] 55,723 pass(es).
[ View ]

I will let xjm RTBC this; the patch is harmless I just see no point.

Assigned:Unassigned» xjm

Assigned:xjm» Unassigned
Status:Needs review» Reviewed & tested by the community

This seems much more sane.

Title:Figure out what the deal is with field_reserved_columns()Move field_reserved_columns() to a static method

Better title. ;)

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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

Issue summary:View changes

Updated issue summary.