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

Not possible to create database triggers with more than one statement in custom migrations #442

Closed
alessandrojean opened this issue Jan 28, 2025 · 6 comments · Fixed by #451
Assignees
Labels
bug Something isn't working

Comments

@alessandrojean
Copy link

Describe the bug
I'm trying to implement full-text search within a project by using SQLite's fts5, which seems to be supported by Cloudflare D1 SQL syntax as well (reference).

One of the ways to don't make the application code be responsible to insert, update and delete the data in the fts indexes tables is to use triggers, as exemplified in the SQLite docs:

-- Create a table. And an external content fts5 table to index it.
CREATE TABLE t1(a INTEGER PRIMARY KEY, b, c);
CREATE VIRTUAL TABLE fts_idx USING fts5(b, c, content='t1', content_rowid='a');

-- Triggers to keep the FTS index up to date.
CREATE TRIGGER t1_ai AFTER INSERT ON t1 BEGIN
  INSERT INTO fts_idx(rowid, b, c) VALUES (new.a, new.b, new.c);
END;
CREATE TRIGGER t1_ad AFTER DELETE ON t1 BEGIN
  INSERT INTO fts_idx(fts_idx, rowid, b, c) VALUES('delete', old.a, old.b, old.c);
END;
CREATE TRIGGER t1_au AFTER UPDATE ON t1 BEGIN
  INSERT INTO fts_idx(fts_idx, rowid, b, c) VALUES('delete', old.a, old.b, old.c);
  INSERT INTO fts_idx(rowid, b, c) VALUES (new.a, new.b, new.c);
END;

Using triggers makes the DB itself to be responsible to update the fts table, which makes it a better approach to don't depend on adding additional queries on each entity in the application that needs to be searchable.

The problem is the NuxtHub custom migration code seems to be breaking the migration file into multiple lines even inside the trigger code, which makes the migration to fail. I've taken a look into the Hub code, and it seems the culprit is the splitSqlQueries function. As the splitSqlQueries splits the migration statements, and it passes each splitted line to a prepared statement, it seems to not work as the prepared statement should include all of the trigger code instead, I assume.

In the end, the migration never gets applied and I always get a "Failed to apply query" error accusing incomplete commands on console.

Digging dipper, I copied the splitSqlQueries into a custom .js file (Gist) and have run it with the SQLite example given above, and it results as follows:

[
  "CREATE TABLE t1(a INTEGER PRIMARY KEY, b, c);",

  "CREATE VIRTUAL TABLE fts_idx USING fts5(b, c, content='t1', content_rowid='a');",

  "CREATE TRIGGER t1_ai AFTER INSERT ON t1 BEGIN\n  INSERT INTO fts_idx(rowid, b, c) VALUES (new.a, new.b, new.c);",

  "END;",

  "CREATE TRIGGER t1_ad AFTER DELETE ON t1 BEGIN\n  INSERT INTO fts_idx(fts_idx, rowid, b, c) VALUES('delete', old.a, old.b, old.c);",

  "END;",

  "CREATE TRIGGER t1_au AFTER UPDATE ON t1 BEGIN\n  INSERT INTO fts_idx(fts_idx, rowid, b, c) VALUES('delete', old.a, old.b, old.c);",

  "INSERT INTO fts_idx(rowid, b, c) VALUES (new.a, new.b, new.c);",

  "END;"
]

As it can be seen, the trigger is break within multiple separated statements to be executed. I've found #372, but even if the entire migration code is in one line, it still returns the same error.

Steps to reproduce
Steps to reproduce the behavior:

  1. Create a custom Drizzle migration with triggers that use multiple statements.
  2. Restart the project so the migrations can be applied.
  3. The migration fail to be applied.

Expected behavior
The migration should apply correctly.

@alessandrojean alessandrojean added the bug Something isn't working label Jan 28, 2025
@RihanArfan RihanArfan self-assigned this Jan 31, 2025
@RihanArfan
Copy link
Collaborator

Thanks for reporting this bug. Could you try out npm i https://pkg.pr.new/nuxt-hub/core/@nuxthub/core@451 and let me know if that solves this?

@alessandrojean
Copy link
Author

Hi, Rihan, sorry for the delay!

I tried using this package, but I am always getting this error:

ERROR  Failed to apply migration .data/hub/database/migrations/0016_add-book-fts5.sql
D1_ERROR: near ";": syntax error at offset 169: SQLITE_ERROR

I tried my Gist again with your modifications to splitSqlQueries, but it seems to be adding a extra semicolon in the end of some statements, which causes the error:

[
  "CREATE TABLE t1(a INTEGER PRIMARY KEY, b, c);",
  "CREATE VIRTUAL TABLE fts_idx USING fts5(b, c, content='t1', content_rowid='a');",
  "CREATE TRIGGER t1_ai AFTER INSERT ON t1 BEGIN\n  INSERT INTO fts_idx(rowid, b, c) VALUES (new.a, new.b, new.c);;\nEND;",
  "CREATE TRIGGER t1_ad AFTER DELETE ON t1 BEGIN\n  INSERT INTO fts_idx(fts_idx, rowid, b, c) VALUES('delete', old.a, old.b, old.c);;\nEND;",
  "CREATE TRIGGER t1_au AFTER UPDATE ON t1 BEGIN\n  INSERT INTO fts_idx(fts_idx, rowid, b, c) VALUES('delete', old.a, old.b, old.c);;\n  INSERT INTO fts_idx(rowid, b, c) VALUES (new.a, new.b, new.c);;\nEND;"
]

All statements inside the triggers seems to be ending with ;;.

Thanks for your time into fixing this, btw!

@RihanArfan
Copy link
Collaborator

Apologies, could you try again now?

@alessandrojean
Copy link
Author

Tried again, the double semicolon issue seems to be fixed, but somehow I am still getting the same error.

 ERROR  Failed to apply migration .data/hub/database/migrations/0016_create-books-fts-table.sql
 D1_ERROR: near ";": syntax error at offset 169: SQLITE_ERROR

For reference, this is the migration code I'm trying to run:

-- Custom SQL migration file, put your code below! --
CREATE VIRTUAL TABLE books_fts USING fts5(title, subtitle, synopsis, content='books', content_rowid='id');

CREATE TRIGGER books_ai AFTER INSERT ON books BEGIN
  INSERT INTO books_fts(rowid, title, subtitle, synopsis)
    VALUES (new.id, new.title, new.subtitle, new.synopsis);
END;

CREATE TRIGGER books_ad AFTER DELETE ON books BEGIN
  INSERT INTO books_fts(books_fts, rowid, title, subtitle, synopsis)
    VALUES('delete', old.id, old.title, old.subtitle, old.synopsis);
END;

CREATE TRIGGER books_au AFTER UPDATE ON books BEGIN
  INSERT INTO books_fts(books_fts, rowid, title, subtitle, synopsis)
    VALUES('delete', old.id, old.title, old.subtitle, old.synopsis);
  INSERT INTO books_fts(rowid, title, subtitle, synopsis)
    VALUES (new.id, new.title, new.subtitle, new.synopsis);
END;

I'm kinda lost here on what the error might be tbh 😅, tried the Gist again, and it seems to be splitting things fine now. I also tried some variants in the code such putting all the trigger code in one line, but no luck either.

@RihanArfan
Copy link
Collaborator

Thanks for all the details. I suspect the issue now is that the @nuxthub/core@451 is locally cached to the old commit. Could you try with this and see?

pnpm i https://pkg.pr.new/nuxt-hub/core/@nuxthub/core@d9e1da0

If it still doesn't work, you could try deleting your node_modules and your package manager lock file, or resetting your local D1 database by deleting .data/hub/d1 then retrying.

I've tried running that SQL (in addition to creating the table first) and it seems to successfully apply. Let me know how it goes :)

@alessandrojean
Copy link
Author

That did the trick! The migration applied successfully now 🥳.

Thanks for your patience and time into fixing this, really appreciate it.

Waiting for the PR to be merged 👍🏻.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants