-
Notifications
You must be signed in to change notification settings - Fork 642
adjustments for knex v1 and v2 #2372
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
adjustments for knex v1 and v2 #2372
Conversation
knex v1+ changed the return values when `.returning()` clause is used (affecting postgres), before it returned array of primitive values if only 1 column was selected to be returned (e.g. `[1]` for `.returning('id')`) but now it always returns objects (e.g. `[{ id: 1 }]` for `.returning('id')`).
Slightly reorganized `InsertOperation`, instead of calling `createModels` in `doExecute()` (right after `onRawResult` hook) and having "empty" model instances created there, possibly with undefined / declared default properties which would later overwrite input models in `InsertOperation.onAfter1`, offload the handling to `InsertOperation.onAfter1`, which now uses also `setDatabaseJson`, same as `createModels` function, to be consistent.
Additionally, adjusted `$parseDatabaseJson` in `GraphInsert` integration tests, to only parse boolean values when they are present in `json`.
related change in knex v1.0: knex/knex#4471
to make shapes of both querybuilders as close to each other as possible. Most of the methods were introduced in knex v1. - `fromRaw` - `whereLike` - `andWhereLike` - `orWhereLike` - `whereILike` - `andWhereILike` - `orWhereILike` - `withMaterialized` - `withNotMaterialized` - `jsonExtract` - `jsonSet` - `jsonInsert` - `jsonRemove` - `whereJsonObject` - `orWhereJsonObject` - `andWhereJsonObject` - `whereNotJsonObject` - `orWhereNotJsonObject` - `andWhereNotJsonObject` - `whereJsonPath` - `orWhereJsonPath` - `andWhereJsonPath` and one new `JoinBuilder` method - `andOnJsonPathEquals` (referencing knex's `JoinClause` method of the same name) `whereJson(Not)SupersetOf` / `whereJson(Not)SubsetOf` are now supported by knex >= 1.0, but for now objection handles them differently and only for postgres. Changing them to utilize knex methods directly may require a major version bump and upgrade guide. Added a test for checking JoinBuilder methods to be inline with knex's JoinClause. Fixed incorrect checking QueryBuilder methods if they are inline with knex's QueryBuilder. Added typings for new methods. Bumped knex `peerDependency` from `>=0.95.13` to `>=1.0.1` because of new methods.
|
I'll publish this as a new semver major after releasing minor tomorrow. |
|
@kibertoad perhaps for a major version bump it would be worth it to rework In this PR the only kind-of-breaking change is minimal knex dependency bump from 0.95 to 1.0.1. While it might still work fine with knex 0.95 unless you start using the new json methods. |
|
@falkenhawk I'm open to using this semver major window for addressing that. Could you please send the PR? |
|
@kibertoad I am not sure if I can make it. Meanwhile I updated knex-related docs in #2375 |
@kibertoad I made an attempt, but as I read through https://vincit.github.io/objection.js/api/query-builder/find-methods.html#wherejsonsupersetof there is an example to reference other json field in a where clause as a value. const people = await Person.query().whereJsonSupersetOf(
'additionalData:myDogs',
'additionalData:dogsAtHome'
);I am not sure if the same is supported in knex, docs does not say anything about it: It would be nice to have those json functions available also for other clients, other than postgres, which knex currently supports, but I am afraid of introducing an unnecessary regression, making it a blocker to update to new version of objection.js for postgres users who currently use those functions that way (referencing json fields as values) Maybe you know more about the internals, or may @OlivierCavadenti (who implemented knex/knex#4859) shed some light here, please? |
|
@kibertoad from my investigations it won't be trivial to switch Even if "field expressions" - https://github.com/Vincit/objection.js/blob/master/lib/queryBuilder/operations/jsonApi/postgresJsonApi.js#L45-L51 - were translated to combinations of column names + In my case it is getting stuck in infinite loops, but maybe I tested it wrong. Otherwise changes to knex codebase would be needed to make it work, i.e. allowing to migrate such json queries while keeping all current features on objection's side. That said, I am not able to dig deeper, as I don't have too much time on my hands currently. I don't want to hold you back there, Feel free to proceed with the release, and in the future, when someone approaches this again, we could have yet another major version bump ;) |
ported from ovos#15
adjustments for knex v1 and v2
knex v1+ changed the return values when
.returning()clause is used (affecting postgres), before it returned array of primitive values if only 1 column was selected to be returned (e.g.[1]for.returning('id')) but now it always returns objects (e.g.[{ id: 1 }]for.returning('id')).Slightly reorganized
InsertOperation, instead of callingcreateModelsindoExecute()(right afteronRawResulthook) and having "empty" model instances created there, possibly with undefined / declared default properties which would later overwrite input models inInsertOperation.onAfter1, offload the handling toInsertOperation.onAfter1, which now uses alsosetDatabaseJson, same ascreateModelsfunction, to be consistent.Additionally, adjusted
$parseDatabaseJsoninGraphInsertintegration tests, to only parse boolean values when they are present injson.related change in knex v1.0: knex/knex#4471
introduced new (or missing) knex querybuilder's methods
to make shapes of both querybuilders as close to each other as possible.
Most of the methods were introduced in knex v1.
fromRawwhereLikeandWhereLikeorWhereLikewhereILikeandWhereILikeorWhereILikewithMaterializedwithNotMaterializedjsonExtractjsonSetjsonInsertjsonRemovewhereJsonObjectorWhereJsonObjectandWhereJsonObjectwhereNotJsonObjectorWhereNotJsonObjectandWhereNotJsonObjectwhereJsonPathorWhereJsonPathandWhereJsonPathand one new
JoinBuildermethod -andOnJsonPathEquals(referencing knex'sJoinClausemethod of the same name)related: knex/knex#4859 knex/knex#5044
whereJson(Not)SupersetOf/whereJson(Not)SubsetOfare now supported by knex >= 1.0, but for nowobjection handles them differently and only for postgres.
Changing them to utilize knex methods directly may require a major version bump and upgrade guide.
Added a test for checking JoinBuilder methods to be inline with knex's JoinClause.
Fixed incorrect checking QueryBuilder methods if they are inline with knex's QueryBuilder.
Added typings for new methods.
Bumped knex
peerDependencyfrom>=0.95.13to>=1.0.1because of new methods.