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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX[Sqlite3] - Date/DateTime columns string value normalization #6022

Open
briosheje opened this issue Mar 4, 2024 · 0 comments
Open

FIX[Sqlite3] - Date/DateTime columns string value normalization #6022

briosheje opened this issue Mar 4, 2024 · 0 comments

Comments

@briosheje
Copy link

Hi! 馃憢

Firstly, thanks for your work on this project! 馃檪

Today I used patch-package to patch knex@3.0.1 for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/knex/lib/dialects/sqlite3/query/sqlite-querycompiler.js b/node_modules/knex/lib/dialects/sqlite3/query/sqlite-querycompiler.js
index 47b282e..1c1e676 100644
--- a/node_modules/knex/lib/dialects/sqlite3/query/sqlite-querycompiler.js
+++ b/node_modules/knex/lib/dialects/sqlite3/query/sqlite-querycompiler.js
@@ -9,6 +9,7 @@ const reduce = require('lodash/reduce');
 const QueryCompiler = require('../../../query/querycompiler');
 const noop = require('../../../util/noop');
 const { isString } = require('../../../util/is');
+const { dateToString } = require('../../../util/string');
 const {
   wrapString,
   columnize: columnize_,
@@ -82,6 +83,13 @@ class QueryCompiler_SQLite3 extends QueryCompiler {
       });
     }
 
+    insertData.values.forEach((values, i) =>
+      values.forEach((val, j) => {
+        if (val instanceof Date) {
+          insertData.values[i][j] = dateToString(val);
+        }
+      })
+    );
     if (insertData.values.length === 1) {
       const parameters = this.client.parameterize(
         insertData.values[0],
@@ -117,6 +125,9 @@ class QueryCompiler_SQLite3 extends QueryCompiler {
       const block = (blocks[i] = []);
       let current = insertData.values[i];
       current = current === undefined ? this.client.valueForUndefined : current;
+
+      if (current instanceof Date) current = dateToString(current);
+      
       while (++i2 < insertData.columns.length) {
         block.push(
           this.client.alias(
@@ -152,6 +163,15 @@ class QueryCompiler_SQLite3 extends QueryCompiler {
   // Compiles an `update` query, allowing for a return value.
   update() {
     const withSQL = this.with();
+    if (this.single.update) {
+      // -- Patch: update entries that are dates so that they are serialized
+      // as strings as intended.
+      Object.entries(this.single.update).forEach(([k, v]) => {
+        if (v instanceof Date) {
+          this.single.update[k] = dateToString(v);
+        }
+      });
+    }
     const updateData = this._prepUpdate(this.single.update);
     const wheres = this.where();
     const { returning } = this.single;

This issue body was partially generated by patch-package.

Abstract about the problem

When using the sqlite3 adapter, the date / datetime values are not written properly to the database.

The patch from #5272 only partially addresses the issue since it only works for inserts but not for updates.

This issue contains a proposal for fixing the issue by adding these changes:

Unfortunately there is not much documentation about such integration and I'm not 100% sure by design you intend to have it working in such direction, so it would be nice to have some discussion about potential issues that this PR would create.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant