Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

queryBuilder comment method does not work for update statements #5775

Open
blaisn opened this issue Dec 20, 2023 · 1 comment
Open

queryBuilder comment method does not work for update statements #5775

blaisn opened this issue Dec 20, 2023 · 1 comment

Comments

@blaisn
Copy link

blaisn commented Dec 20, 2023

Environment

Knex version: 3.1.0
Database postgesql, version: 15.4
OS: Windows 10

Bug

  1. Explain what kind of behaviour you are getting and how you think it should do
    This knex call:
await this.trx("my_table")
    .update(data)
    .where(where)
    .comment("This is a comment in an UPDATE");

where trx is obtained by knex.transaction().
Should give in knex debug:

{
  method: 'update',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [ 'new value', true, 'CENTR' ],
  __knexQueryUid: 'Wlhi38PhCdyhCTnd7-DlW',
  sql: '/* This is a comment in an UPDATE */ update "my_table" set ... where ...  returning: undefined
}

According to the docs:
.comment(comment)
Prepend comment to the sql query using the syntax /* ... */.

While with the SELECT construct:

await this.trx
    .select("*")
    .from("my_table")
    .orderBy(order_by)
    .comment("This is a comment in a SELECT");

we definitely get the comment:

{
  method: 'select',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [],
  __knexQueryUid: '-cu9ehMe_3Y3jGx4Gpy20',
  sql: '/* This is a comment in a SELECT */ select * from "my_table"'
}
  1. Error message
    No error messages

  2. Reduced test code, for example in https://npm.runkit.com/knex or if it needs real
    database connection to MySQL or PostgreSQL, then single file example which initializes
    needed data and demonstrates the problem.

Will be provided if we go further, if I am not mistaken.

Feature discussion / request

  1. Explain what is your use case
    With data protection enforcement regulations, we have to audit the database modifications by users. Since we use a generic db user account for the application to connect, it does not make much sense to record that db user as being accountable for all modifications. We would like to record the application user known by the backend server. To log application user alteration of the db, the only way I have found is to "comment" inserts, updates and deletes with the application user name. That way all DMLs issued by the backend will be logged on db server in the name of the real user responsible for the modification.

  2. Explain what kind of feature would support this
    With the comment method working for all SQL. For example:
    trx.on("start", (queryBuilder) => queryBuilder.comment('Application user: ${$email}'));
    would automatically append the comment with application username to ALL SQL issued by knex.

  3. Give some API proposal, how the feature should work
    Make "comment" work for all DMLs.

  4. Of course, I you have another way to record this information on the server logs, I would appreciate!

@blaisn
Copy link
Author

blaisn commented Jan 29, 2024

Since the present issue was not commented/reeviewd by maintainers, I have created a PR (#5793) to solve this problem for pg diaclect.
A previous PR (#5738 merged) solved this issue for mysql dialect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant