-
Notifications
You must be signed in to change notification settings - Fork 49
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
Bug: Query customization doesn't apply to the count SQL query #120
Comments
Please open bugs with informative description and examples. |
I'm sorry if I'm not clear. Let me explain. I customize the query in my example (this is an undocumented feature, but the documentation says: "It works like the Knex service adapter", and the Knex service adapter has customizing the query feature). But my modifications to the query don't apply to the count SQL query. See logs:
I think the problem may be this: You use const q = params.objection || this.createQuery(params); But in the count query, you don't use const countQuery = this._createQuery(params); |
"It works like the Knex service adapter" - in no point this lib was ment to
be like the Knex service adapter.
This lib was forked from the Knex service lib years ago and was built in a
different way.
Please read the docs (README).
בתאריך שבת, 17 באוק׳ 2020, 15:27, מאת mikeconley12 <
notifications@github.com>:
… I'm sorry if I'm not clear. Let me explain.
I customize the query in my example
<master...mikeconley12:master>
(this is an undocumented feature, but the documentation says: "It works
like the Knex service adapter", and the Knex service adapter has customizing
the query
<https://github.com/feathersjs-ecosystem/feathers-knex#customizing-the-query>
feature).
But my modifications
<https://github.com/mikeconley12/feathers-objection/blob/master/example/app.js#L47>
to the query don't apply to the count SQL query. See logs:
knex:query select count(`todos`.`id`) as `total` from `todos` where `text` = ? undefined +5s
knex:query select `todos`.* from `todos` where `text` = ? and `complete` = ? limit ? undefined +3ms
I think the problem may be this:
You use params.objection here
<https://github.com/feathersjs-ecosystem/feathers-objection/blob/master/src/index.js#L474>
,
const q = params.objection || this.createQuery(params);
But in the count query, you don't use
<https://github.com/feathersjs-ecosystem/feathers-objection/blob/master/src/index.js#L511>
params.objection
const countQuery = this._createQuery(params);
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#120 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABB5E3N6FS62XZGT2EV6RILSLGEUXANCNFSM4SE5VXEQ>
.
|
Thanks for the answers! |
I created a reproduction repository: https://github.com/mikeconley12/feathers-objection .
See the changes: master...mikeconley12:master
Steps to reproduce
git clone https://github.com/mikeconley12/feathers-objection.git
cd feathers-objection/
npm i
DEBUG=knex:query npm run example
In your terminal, you should see the following:
The first SQL query doesn't have the
`complete` = ?
condition, but the second SQL query has this condition.The text was updated successfully, but these errors were encountered: