Refactoring method createTable

[vitaly-t]

Replacing the needless task + transaction chain, by replacing it with just one transaction that encapsulates the complete logic.

---

Ended up having the transaction inside the task, to make it possible to execute the transaction when the task fails.

[codecov]

Codecov Report

Merging #4456 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4456      +/-   ##
==========================================
+ Coverage   92.68%   92.69%   +0.01%     
==========================================
  Files         118      118              
  Lines        8348     8349       +1     
==========================================
+ Hits         7737     7739       +2     
+ Misses        611      610       -1

Impacted Files	Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js	96.92% <100%> (ø)	⬆️
src/Adapters/Auth/meetup.js	84.21% <0%> (-5.27%)	⬇️
src/RestWrite.js	93.64% <0%> (+0.36%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34453bf...c2f00ce. Read the comment docs.

[vitaly-t]

@flovilmart one more nailed 😉

[flovilmart]

quick question, shouldn't we use .bind(this) instead of self capturing?

[vitaly-t]

Not sure how it would work in that context...

[flovilmart]

@vitaly-t

 return conn.task('create-table', function * (t) {
      try {
        yield this._ensureSchemaCollectionExists(t);
        yield t.none(qs, values);
      } catch(error) {
          if (error.code !== PostgresDuplicateRelationError) {
            throw error;
          }
        // ELSE: Table already exists, must have been created by a different request. Ignore the error.
      }
      yield t.tx('create-table-tx', tx => {
        return tx.batch(relations.map(fieldName => {
          return tx.none('CREATE TABLE IF NOT EXISTS $<joinTable:name> ("relatedId" varChar(120), "owningId" varChar(120), PRIMARY KEY("relatedId", "owningId") )', {joinTable: `_Join:${fieldName}:${className}`});
        }));
      });
    }.bind(this));

[flovilmart]

@vitaly-t but not required, I'm fine with the current implementation :)

[vitaly-t]

It is debatable. The thing is, methods .task and .tx on its own provide this equal the t parameter, which would cause a contradiction/conflict, not sure even would even work then.

The alternative approach, using the library's feature is to do .task.call(this,..., and then access the context via t.ctx.context, see TaskContext.

But I just went for the simplest solution that's also known not to cause any conflict ;)

[dplewis]

@vitaly-t Merry Christmas. Quick question. I've notice you got rid of t.batch in favor of t.none(this._pgp.helpers(...)) in other functions. Is there a reason why you use it here.

[vitaly-t]

@dplewis as per that PR, it is a performance optimization, to execute all queries via a single IO operation, as opposed to executing them separately. It is much faster that way ;)

[dplewis]

I always thought .batch was a single I/O operation.

[dplewis]

Looks good to me 👍

[vitaly-t]

batch is an operation irrelevant as to the IO, it simply resolves an array of promises, like Promise.all does, except it also settles all the promises.