Problem/Motivation

NativeJSONItem field sets 'type' => 'json' in its schema definition. In case you have json_native field and running tests with SQLite database this results in an exception:

Undefined array key "json:normal"

/web/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php:142
/web/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php:97
/web/core/lib/Drupal/Core/Database/Driver/sqlite/Schema.php:61
/web/core/lib/Drupal/Core/Database/Schema.php:616
/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorageSchema.php:421

The reason is that json datatype does not exist in SQLite https://www.sqlite.org/datatype3.html.

Steps to reproduce

* Add json_native field to an entity
* Run tests with SQLite database
* Undefined array key "json:normal" message occurs

Proposed resolution

Add 'sqlite_type' => 'text' in NativeJSONItem class.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#13 json_field-n3252426-13.patch2.55 KBDamienMcKenna
#8 3252426-sqlite-json.patch4.12 KBgeek-merlin
#4 3252426-TEST-ONLY-SHOULD-FAIL-ON-SQLITE.patch2.9 KBmfb

Issue fork json_field-3252426

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mbovan created an issue. See original summary.

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented

Ironically NativeBinaryJSONItem has 'type' => 'text' so I hadn't noticed this. I'm not sure if the proper fix is setting sqlite_type or type here :) Setting type would make it work like NativeBinaryJSONItem.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Geeks4Change, MachbarMacher commented
Status: Active » Postponed (maintainer needs more info)

Cross-checking this with a current installation.
I can create a field and it seems to work well.
The DB schema looks strange though, the value column does not show a key.
So yes, looks like still an issue.

sqlite> .schema node__field_w4c_json
CREATE TABLE IF NOT EXISTS "node__field_w4c_json" (
"bundle" VARCHAR(128) NOT NULL DEFAULT '', 
"deleted" INTEGER NOT NULL DEFAULT 0, 
"entity_id" INTEGER NOT NULL CHECK ("entity_id">= 0), 
"revision_id" INTEGER NOT NULL CHECK ("revision_id">= 0), 
"langcode" VARCHAR(32) NOT NULL DEFAULT '', 
"delta" INTEGER NOT NULL CHECK ("delta">= 0), 
"field_w4c_json_value"  NULL DEFAULT NULL, 
 PRIMARY KEY (entity_id, deleted, delta, langcode)
);
CREATE INDEX node__field_w4c_json_bundle ON node__field_w4c_json (bundle);
CREATE INDEX node__field_w4c_json_revision_id ON node__field_w4c_json (revision_id);
mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
Status: Postponed (maintainer needs more info) » Needs work
FileSize
3252426-TEST-ONLY-SHOULD-FAIL-ON-SQLITE.patch2.9 KB

Here's a test-only patch based on the steps to reproduce; should pass on mysql and fail on sqlite.

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
Status: Needs work » Needs review

Ok the test seems to be working. I'm not sure what the preferred fix is so setting to "needs review".

If 'type' => 'json' is supported by any contrib db backends out there, then that should be used here and 'sqlite_type' => 'text' should be added. Or it could be changed to 'type' => 'text' which is how NativeBinaryJSONItem works.

geek-merlin opened merge request !4

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Geeks4Change, MachbarMacher commented
Title: Set sqlite_type to native JSON field » 3252426-sqlite-json

@mfb: Wow, you're a real test hero!

Then here's the fix too, in a MR.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Geeks4Change, MachbarMacher commented
FileSize
3252426-sqlite-json.patch4.12 KB

WTF? No tests on MR? OK, here a patch.

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Geeks4Change, MachbarMacher commented
Title: 3252426-sqlite-json » Set sqlite_type to native JSON field
geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Geeks4Change, MachbarMacher commented
Status: Needs review » Reviewed & tested by the community

Daring to RTBC (although technically i copied a line to the solution).

Test is proven to be red in #4, now green.

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented

There is no issue testing enabled for this project, maybe a maintainer could configure that

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Parent issue: » #2954215: Plan for JSON Field 8.x-1.0

I read through https://www.sqlite.org/json1.html and was surprised to see the following:

SQLite stores JSON as ordinary text. Backwards compatibility constraints mean that SQLite is only able to store values that are NULL, integers, floating-point numbers, text, and BLOBs. It is not possible to add a sixth "JSON" type.

So yes, despite the fact that SQLite supports JSON, unlike MySQL and PostgreSQL it doesn't support a JSON data type.

This looks good, let's add it.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
FileSize
json_field-n3252426-13.patch2.55 KB

Rerolled after #3281633 was committed.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Title: Set sqlite_type to native JSON field » Support sqlite; set sqlite_type to native JSON field
Status: Reviewed & tested by the community » Fixed

Committed. Thank you.

  • DamienMcKenna committed b62621b on 8.x-1.x authored by mfb 
    Issue #3252426 by geek-merlin, mfb, DamienMcKenna, mbovan: Support...

Status: Fixed » Closed (fixed)

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

geek-merlin’s picture

geek-merlin
they/omg
Primary language German
Location Freiburg, Germany
CreditAttribution: geek-merlin at Geeks4Change, MachbarMacher commented
Related issues: +#3369898: SQLite still fails hook_requirements