Query Comments

[davewasmer]

Issue type:

[ ] question
[ ] bug report
[x] feature request
[ ] documentation issue

Would you be open to a PR that added support for "naming" queries? I.e. something like:

Post.createQueryBuilder()
  .comment(`Lookup all the posts in ${ category } category`)
  .where({ category })
  .getSql()

/*
-- Lookup all the posts in foo category
SELECT ...

It would only be stored / used in TypeORM, obviously not submitted to the driver layer.

I've found a few times where I'm trying to diagnose a API call that makes multiple complex queries and is failing due to an SQL error. I end up trying to read the generated SQL and trying to infer where in my code that query came from. It would be nice to be able to label the queries such that I could easily trace them back when the error causes TypeORM to dump getSql() output.

[Diluka]

Since TypeORM is highly influenced by Hibernate. The Named Query is already defined by JPA and been implemented in Hibernate. I suggest change the name of this feature into query comment.

Post.createQueryBuilder()
  .comment(`Lookup all the posts in ${ category } category`)
  .where({ category })
  .getSql()

[Kononnable]

It would only be stored / used in TypeORM, obviously not submitted to the driver layer.

You propose to log this comment only in some TypeOrm functions or add it as comment in sql query(so it will be visible in profilers and similar tools)?

[davewasmer]

I suggest change the name of this feature into query comment.

Sure, happy to go with a different name for the feature. "query comment" or "named query" are both fine by me.

You propose to log this comment only in some TypeOrm functions or add it as comment in sql query(so it will be visible in profilers and similar tools)?

You make a good point - if it's possible to add it as a comment to the sql query directly that would be nice for the reasons you mentioned.

However, I'm not familiar with the driver layer at all, so I'm not sure if that concept makes sense. Do the driver's typically submit a string of SQL over the database connection, such that we can just add a standard SQL comment? Or do they parse the query into some kind of over-the-wire format that ends up stripping out extraneous info (i.e. stripping out comments)?

[davewasmer]

@Kononnable would you be up for a looking a PR for this? Happy to take a stab at it, but don't want to waste effort if the idea will be turned down regardless.

[Kononnable]

Didn't tested it but I think we should be able to add comments to generated queries.
However if your query is failing wouldn't it be easier to just log a callstack? With callstack it should be pretty easy to find the right place. I think using a logger would be the easiest solution for your problem.

typeorm/src/logger/AdvancedConsoleLogger.ts

Lines 33 to 36 in d0e2899

	/**
	* Logs query that is failed.
	*/
	logQueryError(error: string, query: string, parameters?: any[], queryRunner?: QueryRunner) {

[davewasmer]

@Kononnable unfortunately, logging from there isn’t helpful. We only know the query failed when the DB returns an error, meaning it is necessarily async. The call stack from the logging method only traces back to the driver, and no farther.

I suppose once async stack traces land in node v12 and TypeORM starts shipping native async/await in its compiled output, then yes, logging a stacktrace would help.

But even then, I could imagine query comments being useful in certain production environments as well, or when trying to diagnose slow queries even.

Sounds like you’re open to a PR?

[Kononnable]

Oh, I didn't think about async or event-driven scenarios. Stack trace might not be so useful there.
I think possibility of adding comments will be a good idea. They make almost no overhead and it allow us to use all of DBA tools to diagnose performance problems(without comments we still can use such tools, but it would take more time).

[imnotjames]

I'm going to implement this. I've seen this used MANY times not just for general debugging, but also for distributed tracing, and for experimenting with query plans using features like Query plan stability or whatever the SQL Server studio provides - don't remember the name of it. Another big use case is for explicit routing when you're using ProxySQL - by using the regex rules you can route to a specific set of nodes based on the content of a comment.

This is separate from optimizer hints included in the queries, which this won't really support (most of those are comments in the format of /*+ foo */ but interspersed in certain areas of the query)

Work in progress branch can be found here - it's following the .comment("hello world") notation in the query builder.

[JinyanLai]

is it possible to add comment to insert, save, update
thank you