danielmhanover / typeorm-upsert

37 stars 2 forks source link

update upsert #9

Closed lupu60 closed 3 years ago

lupu60 commented 4 years ago
acepace commented 4 years ago

Note your code fails if options is not passed in at all. Fixable with options = options ? options : {}

acepace commented 4 years ago

Additional note, you should cover excluded and all the column names with quote marks otherwise you collide with typeorms habit of creating columns in different cases.

lupu60 commented 4 years ago

I will fix the options issue. I don't really know how to fix the second one. Maybe you can help me.

acepace commented 4 years ago

I ended up also expanding this to handle multiple conflictKeys because my DB structure isn't amazing.

export async function TypeOrmUpsert<T>(
  repository: Repository<T>,
  object: T | T[],
  conflictKey: string | string[],
  options?: {
    keyNamingTransform?: (k: string) => string;
    doNotUpsert?: string[];
  },
): Promise<T[]> {
  options = options ? options : {}
  options.keyNamingTransform = options.keyNamingTransform || ((k) => k);
  const keys: string[] = _.difference(_.keys(_.isArray(object) ? object[0] : object), options.doNotUpsert || []);
  const setterString = keys.map((k) => `"${options.keyNamingTransform(k)}" = "excluded"."${k}"`);
  if (typeof conflictKey !== 'string') {
    conflictKey = conflictKey.map(str => `"${str}"`).join(', ')
  } else {
    conflictKey = `"${conflictKey}"`
  }
  const onConflict = `(${conflictKey}) DO UPDATE SET ${setterString.join(',')}`;
  const qb = repository.createQueryBuilder().insert().values(object).onConflict(onConflict);
  return (await qb.returning('*').execute()).raw;
}
acepace commented 4 years ago

If you like that, or take what you want and improve, then if it doesn't get merged maybe we should fork

lupu60 commented 4 years ago

Alright I will make the changes.

moltar commented 4 years ago

I don't think upsert can work at all.

See my comments here: https://github.com/typeorm/typeorm/issues/1090#issuecomment-615556201

Please write an integration test for that to make sure that it works or doesn't.

From my experience, I could not get it to work.

moltar commented 4 years ago

Also, I know I'm probably biased, but I think my implementation is cleanest so far, except for not having support for arrays. But that can be added. I just didn't want to spend more time on it, since I couldn't even actually get single rows to work.

The key two things that I think are missing from the other implementations is:

  1. Using escape from the driver.
  2. Using user-defined naming strategy.
acepace commented 4 years ago

Works for me so 🤷‍♀️

lupu60 commented 3 years ago

Btw I moved this to my repo: https://github.com/lupu60/nestjs-toolbox/tree/master/packages/typeorm-upsert#readme @moltar you can open an issue here if you want. @acepace you can now install this as an npm package.