Fix: knex withSchema.raw error #14839
Conversation
Codecov ReportBase: 59.33% // Head: 59.76% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #14839 +/- ##
==========================================
+ Coverage 59.33% 59.76% +0.43%
==========================================
Files 1338 1339 +1
Lines 32726 32669 -57
Branches 6199 6189 -10
==========================================
+ Hits 19417 19526 +109
+ Misses 11440 11296 -144
+ Partials 1869 1847 -22
Flags with carried forward coverage won't be shown. Click here to find out more. Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
jhoward1994
force-pushed
the
fix/relations-knex-raw
branch
from
7919860
to
cad6a8d
Compare
packages/core/database/lib/index.js
Outdated
| const schema = this.connection.getSchemaName(); | ||
| const connection = tableName ? this.connection(tableName) : this.connection; | ||
| return schema ? connection.withSchema(schema) : connection; | ||
| return tableName ? this.connection(tableName) : this.connection; |
There was a problem hiding this comment.
Based on what we discussed, I think we should not change the code here and instead use connection instead of getConnection. WDYT?
chore(database): revert getConnection()
| WHERE ${where.join(' OR ')} | ||
| ) AS b | ||
| WHERE b.id = a.id`, | ||
| [joinTableName, ...updateBinding, ...selectBinding, joinTableName, ...whereBinding] |
There was a problem hiding this comment.
I don't think it works. I think you need to do ??.?? and [schemaName, joinTableName, ...] as each argument is either an identifier or a value. ${schemaName}.${joinTableName} would be 2 identifiers at the same time. Did you try it?
I think you can either:
- directly put the value in the query, not using the arguments
- don't handle the schema for now, we will do it in another PR anyway as there are other parts of the app that needs to be fixed with the schema
I would suggest the second options so we can also take the time to do it (not doing it during the rush)
There was a problem hiding this comment.
Apparently that works indeed! https://knexjs.org/guide/ref.html#ref
Good to know. However I would still not do that in that PR for the moment as it would be add some inconsistency
|
@petersg83 then you drop support for running strapi in different schema ? I have strapi setup in supabase, where the default schema is ? |
|
@zodman |
|
@petersg83 2.5.0 |
|
Can you check again ? This one doesn't exist :P |
|
@petersg83 do you have a current fix for this? Do we need to downgrade it? Change something else? This currently leaves our Strapi in a state where we cannot edit any articles in production, any guidance would be greatly appreciated! |
|
Hello @alexghattas :) It's should be fixed in 4.5.2 :) |
Thank you @petersg83, confirming its fixed the issue on our end :)! |
We received a bug report with v4.5.0 for Postgres DBs that are using the
schemaoption inconfig/database.jsWhat does it do?
fixes #14832
How to test it?
Follow instructions in #14832
Related issue(s)/PR(s)
#14832
closes #14887 - Thank you for the work @zodman 🙏, I just had to make a couple of tweaks
Note: we have decided to not handle multiple PG schemas in this fix to avoid inconsistency. Theres are other places in the codebase where a similar fix will be required. They will all be handled together