New feat: Make "returning" optional on inserts

[eduardo-veras]

Hello!

I would like first to check the reason for a behavior and if makes sense to propose a new feature.

Current behavior
On the current version (v3+), there's a condition that set a default identifier to be returned in the case the user doesn't specify one (except for Sqlite and MySql):

objection.js/lib/queryBuilder/operations/InsertOperation.js

Lines 38 to 46 in 8f88965

	onBuildKnex(knexBuilder, builder) {
	if (!isSqlite(builder.knex()) && !isMySql(builder.knex()) && !builder.has(/returning/)) {
	// If the user hasn't specified a `returning` clause, we make sure
	// that at least the identifier is returned.
	knexBuilder = knexBuilder.returning(builder.modelClass().getIdColumn());
	}
	return knexBuilder.insert(this.models.map((model) => model.$toDatabaseJson(builder)));
	}

Scenario
I'm using PostgreSQL in the AWS RDS cluster, so I have one main writer and multiple readers instances, and from a user permission standpoint, we have users that can only write or only read.

The Problem
Defaulting a returning() causes an error when doing a Model.query().insert({ ... });, because the DB user doing the insert doesn't have permissions to query back the table to retrieve the insert record. Also passing any invalid value, such .returning(null) or .returning(undefined) throw different types of errors.

Workaround
Objection query builder has the .onBuildKnex() that's executed after the the InsertOperation onBuildKnex event, so I did the following that solved my problem:

Model.query().onBuildKnex((knexBuilder, objectionBuilder) => {
      let op = objectionBuilder.findOperation(/returning/);
      if(op && op.args.length == 1 && op.args[0] === null){
          objectionBuilder.removeOperation(op);
      }
      return knexBuilder;
}).insert({ ... }).returning(null);

Proposed change
With all being said, do you think creating a condition similar to this inside the InsertOperation onBuildKnex event, makes sense?

I'm not sure if the best way is just by defining some specific argument on the .returning() (like I did on my solution) or creating a new operation like .noReturning() to no even defining the return.

What are your thought on this? :)

FYI: @koskimas

[lehni]

Out of curiosity, I've commented out this line and ran the tests and there are lots of fails in the Postgres integration tests:

// knexBuilder = knexBuilder.returning(builder.modelClass().getIdColumn());

This makes sense since the code only affects the Postgres client due to the if statement. This suggest that insert on SQLite and MySQL return the id by default, and PostgreSQL only returns if explicitly told to do so. Objection.js tries to keep the behavior between the different clients as similar as possible, and it may well be that code internally relies on the ids to be present, e.g. RelationInsertOperation etc.

What happens if you call returning() with no arguments?