Lucid: .returning() is not supported by mysql and will not have any effect.

[jdunsworth]

Starting with lucid version 6.1.4, my create or createMany calls are now returning the following error in console:

.returning() is not supported by mysql and will not have any effect.

The transaction occurs properly despite the error. The error is being thrown by knexJS through Lucid from the look of it. I downgraded to 6.1.3 and the error no longer occurs.

Package version

@adonisjs/lucid version 6.1.4
knex version 0.15.2
mysql version 2.18.1

Node.js and npm version

Node v12.8.0, NPM v6.10.2

Sample Code (to reproduce the issue)

  await TestModel.create({ log_text: 'this is a test' });

[thetutlage]

Yes, knex raises the warning. And it is a warning and not a error, so it can be ignored safely

[terry-lonesski]

Yes, knex raises the warning. And it is a warning and not a error, so it can be ignored safely

Same problem.
Can you please tell how can I ignore this warning?
Those warnings make my own console.log's unreadable (they just get lost in warnings list)

[nichmorgan]

The message is not a warning, but a message in console.log, so, to ignore this:

// start/app.js

// ... //  
console.log = function (msg, ...options) {
  const ignore = '.returning() is not supported by mysql and will not have any effect.'
  if (msg.indexOf(ignore) === -1) {
    console.info(msg, ...options)
  }
}  
// ... //  

[benknight]

Overriding console to filter specific warnings is not a great solution.

I dug into the source to try the fix this and PR myself and gave up because I don't know how to access the db connection config from the Model classes (when I console.log them, the $connection field is set to empty string). Some way or another, the insert methods in the Model classes (Model/index.js and Model/Base.js) need to check the db connection type before chaining a returning() call as not all db types support it: http://knexjs.org/#Builder-returning

For me the reason I believe this needs a fix is because usually when I see a console warning I assume there is a problem with my code. It took me a good chunk of time to determine that the source of the issue is actually coming from this package. Now that I actually know this I can ignore the warning without the anxiety.

[chadhobson]

@benknight You're on the right track. If connection is empty, then it's using the connection specified in config/database. Otherwise it's using the connection specified in the model itself.

Here's a patch-package fix implementing your idea. I'm using mysql so I can't attest to the affects on the other db clients but I am checking for them before deciding to use returning() or not so they should be handled.

I can submit a PR if its going to be accepted, otherwise I won't waste my time.

@adonisjs+lucid+6.2.0.patch

[benknight]

@chadhobson nicely done. The author specifically invited a PR for this here: https://github.com/adonisjs/core/discussions/1280#discussioncomment-41774

So I'd say go ahead and PR it please :D

[chadhobson]

@benknight Well it was almost nicely done. When doing the PR, I realized I had to change this.connection to this.constructor.connection but all is well now. I also updated the patch gist above so you'll want to use the updated patch if you're using the previous patch revision.

[benknight]

@thetutlage @chadhobson I just PR'd a new fix which is a lot simpler. Turns out there was already a monkey patch to filter out these warnings, mysql just needed to be added to the list of non-supporting clients.

[thetutlage]

Published as v6.2.1. Closing the issue. Thanks everyone for bringing it to the notice. @benknight and @chadhobson thanks for the PRs 🙂