Fix Analytic orderBy and partitionBy to follow the SQL documentation #4602
Conversation
This change is required to do more advanced `row_number,dense_rank, or rank` without doing weird workarounds, as you are not allowed to change the sorting order with the current behavior being ascending only.
|
Thank you for your contribution! Could you please add integration test for this? |
|
@wolfcomp Can I help you with writing a test? |
|
Currently writing the tests. Im also adding it into the |
wolfcomp
changed the title
get to many test errors due to not being connected to a database
|
@kibertoad if you could start the workflows believe it should work as i cant see it in my terminal window due to me not being able to setup sql server connections on the network im on currently |
|
Im unsure to why its saying that the formatter is undefined for columnize as the ones that fails seems to have the formatter set. |
|
formater -> formatter |
|
🤦 |
|
Now it should work surprised that I haven't had any errors my self. |
|
Actually there seems to still be an error as in the project that I use this I did function and not object assign on the parameter. |
|
@kibertoad now it should all be fixed from what I can see in my terminal window |
| @@ -7854,6 +7854,29 @@ describe('QueryBuilder', () => { | |||
| ); | |||
| }); | |||
|
|
|||
| it('row_number with object', function () { | |||
There was a problem hiding this comment.
can you also add an integration test?
There was a problem hiding this comment.
I can try to look into it, haven't really written integration tests before, probably will just copy one of the other ones and get it to fit the function
There was a problem hiding this comment.
@kibertoad after looking through the test/integration folder, I can't find anything previously for rowNumber, denseRank, rank in any of the files, so I'm unsure of how I should do it in that case.
There was a problem hiding this comment.
You are right, we only have unit tests for analytic functions now
|
@wolfcomp Are TS types correct now? Could you add a tsd test for this case? |
|
I can check if the ts files are correct if not correct them fast |
|
There seems to be no tsd tests for analytic functions at all from what I can find. |
|
@kibertoad this is what the best I can do from what I can find in the files when it comes to already existing code for testing and documenting knex/documentation#333 the added and corrected functionality |
|
Released in 0.95.11 |
This is a little annoyance I found while writing query calls using
rowNumberas the typescript types shows you are allowed to do this but if you try to use an array of objects the SQL request fail withcolumn ?? does not existwhere ?? is the array of objects columnized.