@hasOne generates undefined field names in SQLite

[jpslav]

Before opening, please confirm:

• I have searched for duplicate or closed issues and discussions.
• I have read the guide for submitting bug reports.
• I have done my best to include a minimal, self-contained set of instructions for consistently reproducing the issue.

JavaScript Framework

React Native

Amplify APIs

DataStore

Amplify Categories

Not applicable

Environment information

# Put output below this line
  System:
    OS: macOS 12.6
    CPU: (10) arm64 Apple M1 Max
    Memory: 306.36 MB / 64.00 GB
    Shell: 5.1.16 - /opt/homebrew/bin/bash
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.16.0/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
  Browsers:
    Chrome: 109.0.5414.87
    Firefox: 107.0
    Safari: 16.1
  npmPackages:
    @aws-amplify/datastore-storage-adapter: ^2.0.10 => 2.0.10 
    @babel/core: ^7.12.9 => 7.20.12 
    @react-native-async-storage/async-storage: ^1.17.11 => 1.17.11 
    @react-native-community/netinfo: ^9.3.7 => 9.3.7 
    HelloWorld:  0.0.1 
    aws-amplify: ^5.0.10 => 5.0.10 
    expo: ~47.0.12 => 47.0.13 
    expo-file-system: ^15.1.1 => 15.1.1 
    expo-sqlite: ^11.0.0 => 11.0.0 
    expo-status-bar: ~1.4.2 => 1.4.2 
    hermes-inspector-msggen:  1.0.0 
    ini: ^1.3.5 => 1.3.8 
    inquirer: ^6.5.1 => 6.5.2 
    react: 18.1.0 => 18.1.0 
    react-native: 0.70.5 => 0.70.5 
  npmGlobalPackages:
    @aws-amplify/cli: 10.6.1
    corepack: 0.10.0
    eas-cli: 0.57.0
    eslint: 8.21.0
    npm: 8.11.0
    yarn: 1.22.19

Describe the bug

When using @hasOne and SQLite, DataStore generates fields named "undefined". When a model has more than one @hasOne relation, this creates two fields in the same model named "undefined" which generates an error:

 WARN  [WARN] 25:38.712 ExpoSQLiteDatabase [Error: Error code 1: duplicate column name: undefined]

Models with only one @hasOne do not generate this error (as they can't have duplicate "undefined" column names), but they still have fields named "undefined" which is probably a bad thing.

Expected behavior

DataStore models with @hasOne relations should not lead to commands to SQLite to make fields named "undefined".

Reproduction steps

This bug has been reproduced in https://github.com/jpslav/amplify-bug-has-one. This is a fresh expo app created with:

npx create-expo-app HasOneIssue
cd HasOneIssue/
npx amplify-app@latest
npm install aws-amplify @aws-amplify/datastore-storage-adapter expo-sqlite expo-file-system @react-native-community/netinfo @react-native-async-storage/async-storage

I then created a very small data model in schema.graphql:

type Car @model {
  id: ID!
  engine: Engine @hasOne
  trunk: Trunk @hasOne
}

type Engine @model {
  id: ID!
}

type Trunk @model {
  id: ID!
}

Then I generated the models with npm run amplify-modelgen. The app configures DataStore to use the ExpoSQLiteAdapter and then calls DataStore.start().

The error is revealed when the app is run with npm start. I chose the iOS Simulator device, but anything that runs the app should show the error.

To get greater insight into the error, I dropped some console.log statements into node_modules/@aws-amplify/datastore-storage-adapter/dist/aws-amplify-datastore-sqlite-adapter-expo.js:

statements = Object(_common_SQLiteUtils__WEBPACK_IMPORTED_MODULE_2__["generateSchemaStatements"])(this.schema);
statements.forEach((statement) => console.log(statement)); // added line
return [4 /*yield*/, this.db.createSchema(statements)];

This logger prints out:

 LOG  CREATE TABLE IF NOT EXISTS "Setting" ("id" PRIMARY KEY NOT NULL, "key" TEXT NOT NULL, "value" TEXT NOT NULL);
 LOG  CREATE TABLE IF NOT EXISTS "Car" ("id" PRIMARY KEY NOT NULL, "engine" TEXT, "undefined" TEXT, "trunk" TEXT, "undefined" TEXT, "createdAt" TEXT, "updatedAt" TEXT, "carEngineId" TEXT, "carTrunkId" TEXT, "_version" INTEGER, "_lastChangedAt" INTEGER, "_deleted" INTEGER);
 LOG  CREATE TABLE IF NOT EXISTS "Engine" ("id" PRIMARY KEY NOT NULL, "car" TEXT, "undefined" TEXT, "createdAt" TEXT, "updatedAt" TEXT, "engineCarId" TEXT, "_version" INTEGER, "_lastChangedAt" INTEGER, "_deleted" INTEGER);
 LOG  CREATE TABLE IF NOT EXISTS "Trunk" ("id" PRIMARY KEY NOT NULL, "car" TEXT, "undefined" TEXT, "createdAt" TEXT, "updatedAt" TEXT, "trunkCarId" TEXT, "_version" INTEGER, "_lastChangedAt" INTEGER, "_deleted" INTEGER);
 LOG  CREATE TABLE IF NOT EXISTS "MutationEvent" ("id" PRIMARY KEY NOT NULL, "model" TEXT NOT NULL, "data" TEXT NOT NULL, "modelId" TEXT NOT NULL, "operation" TEXT NOT NULL, "condition" TEXT NOT NULL);
 LOG  CREATE TABLE IF NOT EXISTS "ModelMetadata" ("id" PRIMARY KEY NOT NULL, "namespace" TEXT NOT NULL, "model" TEXT NOT NULL, "lastSync" INTEGER, "lastFullSync" INTEGER, "fullSyncInterval" INTEGER NOT NULL, "lastSyncPredicate" TEXT);

In the statement that generates the Car table, you can see the fields named "undefined", one after the engine field and one after the trunk field:

"engine" TEXT, "undefined" TEXT, "trunk" TEXT, "undefined" TEXT

These two columns with the same name cause the error. You can also see the Trunk and Engine models also have fields named "undefined".

Note that I'm using Expo, but the problematic code seems to not be limited to the ExpoSQLiteAdapter.

Code Snippet

// Put your code below this line.

Log output

// Put your logs below this line

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

I did some sleuthing and found some inconsistencies between the form of @hasOne schemas in the tests and what is actually generated. I also see a comment that says some code is limited to @belongsTo but the code doesn't actually appear to be applying that limitation. I'll share these below, but unfortunately I don't know what the right resolution is so I can't make a PR to address them.

Difference between hasOne schema in test helper vs what is actually generated

In the datastore-storage-adapter package, a test helper manually defines a schema object, and its HAS_ONE example has the following form:

association: {
  connectionType: 'HAS_ONE',
  associatedWith: 'id',
  targetName: 'profileID',
},

However, in my reproduction repo, the generated schema for a hasOne looks different:

"association": {
    "connectionType": "HAS_ONE",
    "associatedWith": [
        "car"
    ],
    "targetNames": [
        "carEngineId"
    ]
}

The tests pass but I think that's because the code (see below) is looking for the structure in the test helper schema. When given the structure in the generated schema, it leads to undefined values which get turned into "undefined" string field names.

The BELONGS_TO comment that is probably not reflected in code

The function that builds the SQL statements, modelCreateTableStatement, is In packages/datastore-storage-adapter/src/common/SQLiteUtils.ts. The block of code that handles model fields is where the undefined comes in, I believe.

This block of code has an if that says:

// add targetName as well as field name for BELONGS_TO relations
if (isTargetNameAssociation(field.association)) {

There's another block of code in packages/datastore/src/sync/processors/mutation.ts that has the same isTargetNameAssociation check but that also explicitly checks for a belongsTo:

if (
  isTargetNameAssociation(association) &&
  association.connectionType === 'BELONGS_TO'
) {

which makes me wonder if the check in SQLiteUtils.ts should have the same check.

In any event, I think the if check passes for my @hasOne example (the type guard defined here only checks to see if obj.targetName || obj?.targetNames which is true for either the test helper schema or the real-life schema) . Then the code looks for a foreign key in the model, using the schema present in the test helper schema but not the real-life schema, field?.association?.targetName. That test fails so it then uses the value of field.association.targetName (which is undefined) to as the name of a new field in the SQL statement.

[jpslav]

This comment in packages/datastore/src/utils.ts suggests that datastore-storage-adapter needs to be updated to handle a targetNames array:

 /* Backwards-compatability for schema generated prior to custom primary key support:
the single field `targetName` has been replaced with an array of `targetNames`.
`targetName` and `targetNames` are exclusive (will never exist on the same schema) */

Though I didn't show it in the reproduction repo, @BelongsTo associations also generate "undefined" field names, because those associations also use an array of targetNames but the code in datastore-storage-adapter only knows about the old way of targetName.

[jpslav]

This patch seems to fix things for me. Not knowing when targetNames could have more than one entry, ymmv:

diff --git a/node_modules/@aws-amplify/datastore-storage-adapter/dist/aws-amplify-datastore-sqlite-adapter-expo.js b/node_modules/@aws-amplify/datastore-storage-adapter/dist/aws-amplify-datastore-sqlite-adapter-expo.js
index 49df1ec..0eaa269 100644
--- a/node_modules/@aws-amplify/datastore-storage-adapter/dist/aws-amplify-datastore-sqlite-adapter-expo.js
+++ b/node_modules/@aws-amplify/datastore-storage-adapter/dist/aws-amplify-datastore-sqlite-adapter-expo.js
@@ -1343,12 +1343,12 @@ function modelCreateTableStatement(model, userModel) {
         // check if this field has been explicitly defined in the model
         var fkDefinedInModel = Object.values(model.fields).find(function (f) {
           var _a;
-          return f.name === ((_a = field === null || field === void 0 ? void 0 : field.association) === null || _a === void 0 ? void 0 : _a.targetName);
+          return f.name === ((_a = field === null || field === void 0 ? void 0 : field.association) === null || _a === void 0 ? void 0 : _a.targetName || _a.targetNames[0]);
         });
         // if the FK is not explicitly defined in the model, we have to add it here
         if (!fkDefinedInModel) {
           var required = field.isRequired ? ' NOT NULL' : '';
-          columnParam_2 += ", \"" + field.association.targetName + "\" TEXT" + required;
+          columnParam_2 += ", \"" + field.association.targetName || field.association.targetNames[0] + "\" TEXT" + required;
         }
       }
       // ignore isRequired param for model fields, since they will not contain

[dpilch]

Hi @jpslav, the reason you are seeing this error is because the SQLite adapter has not been updated to support custom primary key. Your patch will probably work as long as you are not trying to set a custom primary key. Another option would be to disable custom primary key entirely by setting features.graphqltransformer.respectprimarykeyattributesonconnectionfield in amplify/cli.json to false. More recent version of the CLI will set this to true when creating a new project.

Sorry for the confusion! We are probably missing proper documentation on this and I will work to remedy this.

[dpilch]

I've opened a public issue to track the progress of custom primary key on SQLite #11017. If you are still facing issue after setting respectprimarykeyattributesonconnectionfield to false please comment.