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

[BUG]: jsonb always inserted as a json string when using postgres-js. #724

Closed
RyanDurk opened this issue Jun 6, 2023 · 38 comments · Fixed by #1785
Closed

[BUG]: jsonb always inserted as a json string when using postgres-js. #724

RyanDurk opened this issue Jun 6, 2023 · 38 comments · Fixed by #1785
Assignees
Labels
bug Something isn't working

Comments

@RyanDurk
Copy link

RyanDurk commented Jun 6, 2023

What version of drizzle-orm are you using?

0.26.5

What version of drizzle-kit are you using?

0.18.1

Describe the Bug

Inserting an object into a postgres jsonb field with db.insert only inserts a string when using the postgres-js.adapter.

Expected behavior

With the pg package, an object is inserted using the code below, which is the expected behavior.

With the postgres-js package, a string is inserted into the table using the same code.

Environment & setup

drizzle packages as above, and, "pg": "8.11.0" and "postgres": "3.3.5"

schema.ts:
import { pgTable, jsonb } from "drizzle-orm/pg-core";

export const logs = pgTable("log", {
line: jsonb("line").$type(),
});

load.ts:

let lines: { line: object }[] = [];
let n = 0;

for await (const line of rl) {
const lineObj = JSON.parse(line);
lines.push({ line: lineObj });

if (++n > numLines) {
  await runFunction(lines);
  lines = [];
  n = 0;
}

}

await runFunction(lines);

runFunction: async (lines) => {
await db.insert(logs).values(lines);
}

@RyanDurk RyanDurk added the bug Something isn't working label Jun 6, 2023
@LeonAlvarez
Copy link

I was working on a fix there, its kinda stale as would need to have different behavior for pg as it expects arrays to be passed as a string #666

@johanneskares
Copy link

same issue here

@huweihong
Copy link

same issue here +1

@benjamin-brady
Copy link

benjamin-brady commented Jun 30, 2023

+1 - json/jsonb shouldn't be listed in the docs as a supported type when this bug exists.

My workaround was to use a db.execute and pass objects into the sql`` template.

const statement = sql`
        INSERT INTO wikidata_article (wikidata_id, category, grade, en_raw_length, en_url_title, labels, sitelinks)
        VALUES (${articleDetails.wikiDataId}, ${articleDetails.category}, ${articleDetails.grade}, ${articleDetails.enBytes}, ${articleDetails.enUrlTitle}, ${articleDetails.labels}, ${articleDetails.sitelinks})
        ON CONFLICT (wikidata_id) DO UPDATE
        SET 
            category = ${articleDetails.category},
            grade = ${articleDetails.grade},    
            en_raw_length = ${articleDetails.enBytes},
            en_url_title = ${articleDetails.enUrlTitle},
            labels = ${articleDetails.labels},
            sitelinks = ${articleDetails.sitelinks}
        `;
 await db.execute(statement);

@BenChaimowicz
Copy link

same issue. :(

@braden-w
Copy link

braden-w commented Jul 20, 2023

My workaround was to use a db.execute and pass objects into the sql`` template.

This works well as a temporary solution!

You could also do a hybrid approach where you call

...
await db
    .insert(table)
    .values({
    id: id,
...
})
...

and then call

await db.execute(
    sql`UPDATE table set property = ${property} WHERE id = ${id}`
)

afterward to handle the value that's jsonb. Note that because this would be two separate calls, I would only recommend this if you're inserting a lot of columns at once/constantly changing your schema and want to maximize type safety (if you rename your columns in schema.ts, it would update in the db.insert syntax but not db.execute(sql... syntax).

@quirm
Copy link

quirm commented Jul 25, 2023

Is not arbitrary string failing to be detected as json/b in postgres? I've stumbled over this today as I've realized that somehow all values are escaped and the entire object is treated as a scalar. I have this workaround with sql helper.

const content = { title: 'test', ... };

// before
await db
  .update(Stories)
  .set({ content })
  .where(eq(Stories.id, story.id));

// after
await db
  .update(Stories)
  .set({
    content: sql`${content}::jsonb`,
  })
  .where(eq(Stories.id, story.id));

Looking at the complexity of postgres json handling, one can appreciate drizzle design to go down to plain sql/functions without a need to have it abstracted or non-existent as other "ORMs". I end up doing something like this:

...set({
  content: sql`jsonb_set(content, '{title}', content->'title' || '{"text": "Hello There"}'::jsonb)`
})

@juanvilladev
Copy link

I too was able to bypass the issue with this bug by wrapping my values call with sql``:

    const product = await tx
      .insert(products)
      .values({
        entityId: input.entityId,
        eventType: input.eventType,
        payload: sql`${input.payload}::jsonb`,
      })

@rverton
Copy link

rverton commented Aug 28, 2023

The workarounds discussed here will fails when using an array instead of an object. My column type is []::jsonb.

This is the code I am using:

  const example = {identifier: "xyz", hostnames: ["foo.bar.com", "bar.foo.com"]}
  await db.insert(assets).values(input).onConflictDoUpdate({
    target: assets.identifier,
    set: input,
  });

  return db.execute(
    sql`UPDATE assets SET hostnames = ${input.hostnames} WHERE identifier = ${input.identifier}`,
  );

The constructed query looks like this, which is of course wrong because it is using one argument for each array element:

Query: UPDATE assets SET hostnames = ($1, $2)::jsonb WHERE identifier = $3 -- params: ["foo.bar.com", "bar.foo.com", "xyz"]
PostgresError: cannot cast type record to json

Any suggestions?

Edit: Using this custom jsonb type fixed it for me: #666 (comment)

@pthieu
Copy link

pthieu commented Sep 3, 2023

@rverton same issue with me
I guess the temp workaround is to use an object instead =\

@wzulfikar
Copy link

I used raw sql but it didn't work for me because it stores the array as object. Found a work around using custom type here and it worked for me. I don't need to use raw sql, and the array will still get stored as array.

smayya337 added a commit to smayya337/duosmium-nextjs that referenced this issue Oct 8, 2023
@StarpTech
Copy link

Any updates? Do you accept a PR?

@xlc
Copy link

xlc commented Nov 12, 2023

I am curious why this was implemented in such way at first place and have not been patched to be the expected beahviour for months.

@xlc
Copy link

xlc commented Nov 14, 2023

a better workaround #666 (comment)

@VladBrok
Copy link

In case someon

+1 - json/jsonb shouldn't be listed in the docs as a supported type when this bug exists.

My workaround was to use a db.execute and pass objects into the sql`` template.

const statement = sql`
        INSERT INTO wikidata_article (wikidata_id, category, grade, en_raw_length, en_url_title, labels, sitelinks)
        VALUES (${articleDetails.wikiDataId}, ${articleDetails.category}, ${articleDetails.grade}, ${articleDetails.enBytes}, ${articleDetails.enUrlTitle}, ${articleDetails.labels}, ${articleDetails.sitelinks})
        ON CONFLICT (wikidata_id) DO UPDATE
        SET 
            category = ${articleDetails.category},
            grade = ${articleDetails.grade},    
            en_raw_length = ${articleDetails.enBytes},
            en_url_title = ${articleDetails.enUrlTitle},
            labels = ${articleDetails.labels},
            sitelinks = ${articleDetails.sitelinks}
        `;
 await db.execute(statement);

This solution works great, but I had a problem with it because I insert an array into jsonb field and for some reason when I passed this array and it had 2+ elements, the elements were destructured, i.e.:

query:

const array_for_jsonb_field = [{"key1": 1}, {"key2": 2}]
sql`INSERT INTO "my_table" VALUES (${id}, ${val}, ${array_for_jsonb_field}`)

result:

INSERT INTO "my_table" VALUES ($1, $2, ($3, $4)) // $3 and $4 should be a single array but this array with 2 elements got destructured into 2 params

In case someone encounters the same problem, use can do the following:

const array_for_jsonb_field = [{"key1": 1}, {"key2": 2}]
sql`INSERT INTO "my_table" VALUES (${id}, ${val}, ${new Param(array_for_jsonb_field)}`) // I wrapped array with `new Param` 

result:

INSERT INTO "my_table" VALUES ($1, $2, $3)

@MariuzM
Copy link

MariuzM commented Mar 25, 2024

My use case:

Table Schema

availability: jsonb('availability')

API Response
image

Table Plus
image

But if i use

availability: sql`${req.body.availability}::jsonb` as AvailabilityT

Then API Resonse is same but Table Plus

image

Using this #666 (comment) also fixed issue can confirm

@MariuzM
Copy link

MariuzM commented Mar 29, 2024

Just wanted to add above when i use this

export const customJsonb = customType<{ data: any }>({
  dataType() {
    return 'jsonb';
  },
  toDriver(val) {
    console.log('🚀 ~ toDriver:', val);
    return val as any;
  },
  fromDriver(value) {
    console.log('🚀 ~ fromDriver', value);
    if (typeof value === 'string') {
      try {
        return JSON.parse(value) as any;
      } catch {}
    }
    return value as any;
  },
});

It does not sort object in same order as i passed, this happens under the fromDriver

image

Using sql same issue

sql`${availability}::jsonb` as AvailabilityT
image

And then API res is also messed up
image

@jckw
Copy link

jckw commented Mar 29, 2024

I just contributed to the bounty on this issue.

Each contribution to this bounty has an expiry time and will be auto-refunded to the contributor if the issue is not solved before then.

Current bounty reward

To make this a public bounty or have a reward split, the maintainer can reply to this comment.

@ovx
Copy link

ovx commented Apr 2, 2024

I just contributed to the bounty on this issue:

https://until.dev/bounty/drizzle-team/drizzle-orm/724

The current bounty for completing it is $70.00 if it is closed within 27 days, and decreases after that.

Others can also contribute to the bounty to increase it.

@pfurini
Copy link

pfurini commented Apr 3, 2024

Just wanted to add above when i use this

export const customJsonb = customType<{ data: any }>({
  dataType() {
    return 'jsonb';
  },
  toDriver(val) {
    console.log('🚀 ~ toDriver:', val);
    return val as any;
  },
  fromDriver(value) {
    console.log('🚀 ~ fromDriver', value);
    if (typeof value === 'string') {
      try {
        return JSON.parse(value) as any;
      } catch {}
    }
    return value as any;
  },
});

It does not sort object in same order as i passed, this happens under the fromDriver

image Using `sql` same issue
sql`${availability}::jsonb` as AvailabilityT
image And then API res is also messed up image

@MariuzM I don't think this is an issue, but it's by design. The JSONB stores JSON data as a binary representation of the JSONB value, which eliminates whitespace, duplicate keys, and key ordering. If key ordering is important for you, use JSON instead, but you'll lose all the other benefits of JSONB.

@LeonAlvarez
Copy link

Just wanted to add above when i use this

export const customJsonb = customType<{ data: any }>({
  dataType() {
    return 'jsonb';
  },
  toDriver(val) {
    console.log('🚀 ~ toDriver:', val);
    return val as any;
  },
  fromDriver(value) {
    console.log('🚀 ~ fromDriver', value);
    if (typeof value === 'string') {
      try {
        return JSON.parse(value) as any;
      } catch {}
    }
    return value as any;
  },
});

It does not sort object in same order as i passed, this happens under the fromDriver

image Using `sql` same issue
sql`${availability}::jsonb` as AvailabilityT
image And then API res is also messed up image

@MariuzM as indicated by @pfurini its postgres jsonb design.

By contrast, jsonb does not preserve white space, does not preserve the order of object keys, and does not keep duplicate object keys. If duplicate keys are specified in the input, only the last value is kept.

https://www.postgresql.org/docs/16/datatype-json.html

@arjunyel
Copy link
Contributor

This patch-package is working for me, from #1785

diff --git a/node_modules/drizzle-orm/postgres-js/driver.js b/node_modules/drizzle-orm/postgres-js/driver.js
index 7e48e8c..219e0a0 100644
--- a/node_modules/drizzle-orm/postgres-js/driver.js
+++ b/node_modules/drizzle-orm/postgres-js/driver.js
@@ -12,6 +12,8 @@ function drizzle(client, config = {}) {
     client.options.parsers[type] = transparentParser;
     client.options.serializers[type] = transparentParser;
   }
+  client.options.serializers['114'] = transparentParser;
+  client.options.serializers['3802'] = transparentParser;
   const dialect = new PgDialect();
   let logger;
   if (config.logger === true) {

@cobbvanth
Copy link

This is urgently needed to be fixed

@BorisChumichev
Copy link

BorisChumichev commented Jun 27, 2024

I just wanted to point out that this issue breaks creating and editing jsonb fields via drizzle-kit studio.

☝️ Those of you who applied sql helper workaround should be careful manipulating data through studio.

@gczh
Copy link

gczh commented Jul 11, 2024

I too was able to bypass the issue with this bug by wrapping my values call with sql``:

    const product = await tx
      .insert(products)
      .values({
        entityId: input.entityId,
        eventType: input.eventType,
        payload: sql`${input.payload}::jsonb`,
      })

Out of curiosity, would this not introduce a possibility for SQL injection? Or does doing ::jsonb coerces the data into jsonb before running whatever that is in input.payload through the sql function?

@tonyxiao
Copy link

There has got to be a solution. Major version change if needed but the current state is really broken!

@cobbvanth
Copy link

@tonyxiao Agreed, this is a major issue that needs to be addressed

@Acorn221
Copy link

Any updates on this?

@demirtasdurmus
Copy link

I resolved this issue by writing a custom wrapper which handles jsonb conversion regardless of the shape of the data.
Here is the link of my public gist.

@AliyanAamir
Copy link

I resolved this issue by writing a custom wrapper which handles jsonb conversion regardless of the shape of the data. Here is the link of my public gist.

Finally after hours of finding a good solution. Yours worked for me. Cheers!

@demirtasdurmus
Copy link

I resolved this issue by writing a custom wrapper which handles jsonb conversion regardless of the shape of the data. Here is the link of my public gist.

Finally after hours of finding a good solution. Yours worked for me. Cheers!

Glad that it works. Cheers!

@tonyxiao
Copy link

tonyxiao commented Aug 7, 2024

Can this be closed now that #1785 is merged?

@AndriiSherman
Copy link
Member

It was merged and fixed by patching a client you are providing to drizzle.

It's important that you do not use a driver client outside of Drizzle because we apply some mapping patches to it. In the upcoming updates, Drizzle will create its own clients and expose them. Additionally, we will have an updated mapping strategy, so we won't patch the clients you provide to Drizzle

It is available in drizzle-orm@beta and will remain in beta for some time (possibly a few days) before being released as the latest version

Before latest I will prepare a guide on how to fix an existing database and will reuse and mention some of the comments from this issue and a PR I've merged

@AndriiSherman
Copy link
Member

Should be fixed in drizzle-orm@0.33.0
Please check release notes before updating

@BenMusch
Copy link

Out of curiosity, would this not introduce a possibility for SQL injection? Or does doing ::jsonb coerces the data into jsonb before running whatever that is in input.payload through the sql function?

@gczh the sql operator sanitizes the input: https://orm.drizzle.team/docs/sql

Any tables and columns provided to the sql parameter are automatically mapped to their corresponding SQL syntax with escaped names for tables, and the escaped table names are appended to column names.
Additionally, any dynamic parameters such as ${id} will be mapped to the $1 placeholder, and the corresponding values will be moved to an array of values that are passed separately to the database.
This approach effectively prevents any potential SQL Injection vulnerabilities.

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