Objection.js: QueryBuilder.toString() and QueryBuilder.toSql() throw TypeError when using JoinEagerAlgorithm

[pvatterott]

In objection 0.7.9, when calling QueryBuilder.toString() or QueryBuilder.toSQL(), objection will throw an exception from RelationJoinBuilder.js.

The following code snippet should be a minimal repro:

try {
  const query = MyModel.query().eager('relation').eagerAlgorithm(MyModel.JoinEagerAlgorithm);
  console.log(query.toString()); // or toSQL();
} catch (err) {
  console.error(err);
}

throws the following exception:

TypeError: Cannot read property 'columns' of undefined
        at RelationJoinBuilder.buildSelects (.../packages/models/node_modules/objection/lib/queryBuilder/operations/eager/RelationJoinBuilder.js:392:37)
        at RelationJoinBuilder.doBuild (.../packages/models/node_modules/objection/lib/queryBuilder/operations/eager/RelationJoinBuilder.js:293:12)
        at RelationJoinBuilder.build (...packages/models/node_modules/objection/lib/queryBuilder/operations/eager/RelationJoinBuilder.js:140:10)
        at JoinEagerOperation.onBeforeBuild (.../packages/models/node_modules/objection/lib/queryBuilder/operations/eager/JoinEagerOperation.js:81:22)
        at QueryBuilder.buildInto (.../packages/models/node_modules/objection/lib/queryBuilder/QueryBuilderOperationSupport.js:315:10)
        at _build (.../packages/models/node_modules/objection/lib/queryBuilder/QueryBuilder.js:1407:25)
        at QueryBuilder.build (.../packages/models/node_modules/objection/lib/queryBuilder/QueryBuilder.js:734:23)
        at QueryBuilder.toString (.../packages/models/node_modules/objection/lib/queryBuilder/QueryBuilder.js:536:17)

[koskimas]

Whoops! Actually there is no way to return anything useful from toString of a join eager query since the table information (all columns names and types) need to be fetched first. toString will work after the first query thought. I need to add a better error message there.

You can chain .debug() method to the query to get the sql printed out during execution. Will that work?

[pvatterott]

.debug() is definitely sufficient for my immediate needs. However, there are a few places where I log a query to our logfile at debug level. The toString() method should at least not throw - I should be able to call toString() on anything without breaking execution (logging should be a benign operation).

[newhouse]

+1 as I also recently experienced this "cannot read property 'columns' of undefined" error while calling 'toString()' on a query and was not sure what was going on. My case was in Objection 0.6(.2?).

[musou1500]

toString should execute synchronously, but fetchColumnInfo fetches column info asynchronously.

Is it possible to process it synchronously ?
I think get column info from jsonschema definition in model.

[koskimas]

jsonSchema is optional and it doesn't necessarily match the database schema. It cannot be used.

[koskimas]

@pvatterott Would you rather have a toString method that happily returns incorrect SQL rather than throws? Isn't a log message with the wrong SQL just confusing?

This will always work:

Person
  .query()
  .eager('something')
  .runAfter((result, builder) => console.log(builder.toString()))

[pvatterott]

@koskimas - ha, not a great set of choices. I'm not sure what the right route is here, but here are some thoughts:

• I'll definitely use the runAfter pattern for logging queries in the future - that definitely makes this issue less critical for me.
• I would expect a toString method to never throw. I'm not sure if this is a bad assumption on my part? I can't find anything in the spec explicitly prohibiting it.
• Incorrect SQL is obviously also confusing - could you maybe return incomplete SQL instead?

Once again, not sure what the best route is/will defer to your expertise. The runAfter pattern will definitely help me out in the future though, thanks for the follow up.

[devinivy]

@koskimas thank you, this is great!