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

[v4] sort by graphql duplicating rows #11892

Open
GautierDele opened this issue Dec 13, 2021 · 30 comments · May be fixed by #20124 or #20125
Open

[v4] sort by graphql duplicating rows #11892

GautierDele opened this issue Dec 13, 2021 · 30 comments · May be fixed by #20124 or #20125
Assignees
Labels
issue: bug Issue reporting a bug severity: high If it breaks the basic use of the product source: core:database Source is core/database package status: confirmed Confirmed by a Strapi Team member or multiple community members

Comments

@GautierDele
Copy link

Bug report

Describe the bug

Graphql sort on nested relations is basically duplicating rows as a join would do.
This was working in v3.

Steps to reproduce the behavior

Create your graphql query on a resource that has a relation. This is a one to many in this case, this should work for any relation i think
image

Call this in your sort parameters:
image

Expected behavior

Should only have one row per item but this is duplicated:
image

As you can see there i have multiples rows with the same id depending on the number of nested articles linked to the category.

System

  • Node.js version: 14.18.1
  • NPM version: 6.14.15
  • Strapi version: 4.0.0
  • Database: MySQL
  • Operating system: Windows 11
@iicdii
Copy link
Contributor

iicdii commented Dec 14, 2021

Is it the same when you request data through REST API?

@GautierDele
Copy link
Author

Yes it is:
image

@derrickmehaffy
Copy link
Member

I don't believe this is possible as I'm looking at the sort code and I believe sort can only be at the parent level (you can sort the relational data itself but you can't sort the parent data on the relational responses.)

Looping in @strapi/maintainers here as I'm honestly not sure.
(Tested and validated the above doesn't work, nor does something like this:

image

@derrickmehaffy derrickmehaffy added this to To be reviewed (Open) in Developer Experience - Old via automation Dec 21, 2021
@GautierDele
Copy link
Author

This was working in v3 and is something I think really usefull when you want to avoid multiple queries

@alexandrebodin
Copy link
Member

Nested sorting should work. There seems to be an issue with joins that aren't distinct for some reason. that's a valid bug

@derrickmehaffy derrickmehaffy added issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: core:database Source is core/database package status: confirmed Confirmed by a Strapi Team member or multiple community members and removed issue: help wanted source: core:strapi Source is core/strapi package labels Dec 22, 2021
@GautierDele
Copy link
Author

@derrickmehaffy do we have anything new on this please ?

@derrickmehaffy
Copy link
Member

I haven't tested in latest versions but nothing new

@GautierDele
Copy link
Author

Can confirm this is still hapening in 4.0.6

image

@derrickmehaffy derrickmehaffy added severity: high If it breaks the basic use of the product and removed severity: medium If it breaks the basic use of the product but can be worked around labels Jan 31, 2022
@maccomaccomaccomacco maccomaccomaccomacco moved this from To be reviewed (Open) to Reviewed in Developer Experience - Old Feb 24, 2022
@Convly Convly self-assigned this Feb 25, 2022
@maccomaccomaccomacco maccomaccomaccomacco moved this from Reviewed to In progress in Developer Experience - Old Feb 25, 2022
@GautierDele
Copy link
Author

Hello @Convly,

Do you have anything new on this maybe ? Thanks :)

@Convly
Copy link
Member

Convly commented Mar 15, 2022

Hey, it's definitely something that I have in my backlog. I'll try to get at it as soon as I can, even though it'll probably be not before ~one or two weeks. Sorry for the time it's taking, I know this is quite an annoying one.

@GautierDele
Copy link
Author

Ok thanks for the feedback ! have a great week

@GautierDele
Copy link
Author

Hello, Anything new on this maybe ?

@Convly
Copy link
Member

Convly commented May 9, 2022

Hey, sorry for the time it's taking and for the time it took me to answer you, I've been dealing with Typescript support for the last month and it left very little time for anything else. It's definitely something I've on my radar but I cannot promise any ETA right now. Really sorry for this situation. I'll keep you updated when I'll work on it.

@ReyRod
Copy link

ReyRod commented May 9, 2022

This is happening on my end too, had to manually sort in the frontend

@xelinorg
Copy link

xelinorg commented Jun 1, 2022

I have tested #13387 but it looks like it is breaking filtering on related field.

What I have tried is to normalize joins and it is working for the duplication problem but is not sorting at all the nested items.

Here on the join helper I have created a function where I am using joinTable and qb.state.joins to fish out an alias that could satisfy the query and avoid running the createPivotJoin function that is adding more alias and joins that potentially are causing the issue.

return createPivotJoin(qb, joinTable, alias, tragetMeta);

So I have changed the code in line 51 of the join helper into something like this...

return existing(joinTable, qb.state.joins) || createPivotJoin(qb, joinTable, alias, tragetMeta);

The function looks like this

const existing = (nextJoin, stateJoins) => {
  const foundPivot = stateJoins.find(j => j.referencedTable === nextJoin.name
    && j.referencedColumn === nextJoin.joinColumn.name
    && j.rootColumn === nextJoin.inverseJoinColumn.referencedColumn
  )
  if (foundPivot && foundPivot.alias ) {
    const usable = stateJoins.find(j => j.rootTable === foundPivot.alias
      && j.rootColumn === nextJoin.inverseJoinColumn.name
      && j.referencedColumn === nextJoin.inverseJoinColumn.referencedColumn
    )
    return usable && usable.alias ? usable.alias: null;
  }
};

The code I have created is pretty ugly and that is because I do not know if my logic is correct in order to make it better.

I am struggling to understand how strapi is handling relations and nested items. I am logging all the queries and I can see that is doing two queries when there is a relation and I have not find out yet how it is creating the nested data and if it needs the data to be flat or not.

Another issue that I can see is the use of distinct in cases that should be using groupBy, Actually I believe that the groupBy is never used and there is no code that will drive execution to the place that will create grouping at the sql level through the query builder.

I hope that I am not causing any confusion as I am trying to help. I have some spare time and I will continue looking into this and all the related issues hoping that someone will eventually solve that.

@xelinorg
Copy link

xelinorg commented Jun 3, 2022

The previous attempt is missing some things... is hard to test manually all the cases if there are not automated test to tell you what is effected by the changes. This needs groupBy if there is no filtering on the related field something similar with solution on #13387 given by @Bassel17

But in general the inner sorting needs more work at least when populating, so I had to add some more ugly code to try out a fix for the many to many case. I have read most of the database package and I can not find a better way to fix the sql queries as should be in my view. I do not know if what I have done is breaking something else that I have not tested.

Take a look a the diff here 48d3e0e

Is messy because the rest of the codebase needs some refactoring to be more friendly for an engineer to give a better solution without having to do crazy things.

I am not going to make a pull request because this needs more work to be sane. I am trying to give you some hints about what I see while I have been digging on some of the database issues you have open. If you want you can check for yourselves and evaluate if this direction could give some solutions to the databases related problems.

@GautierDele
Copy link
Author

We are nearly coming to a year of this issue, is this fixed by any other way right now or still there ?

@akasummer
Copy link

@GautierDele Unfortunately, still happening on the latest version...

@GautierDele
Copy link
Author

@akasummer for almost a year i forgave, this wont be fixed for such a major problem. Either you find an alternative or you find another solution

@taker93
Copy link

taker93 commented Jan 21, 2023

This is also a problem with the REST API, not only GraphQL.

I'd call it a gamebreaker...

Steps for me to reproduce:
Have a "post" with "categories"
Call sth. like "/api/posts?pagination[limit]=4&sort=categories.id"
A post that has just one category will return the post just once.
A post that has two categories will return just once.
A post that has at least three categories will return multiple times.

Edit: Just to make things clear… What i really would like to do is sorting my posts by its number of categories. But there is no sorting option for that why i used to make it work with categories.id in strapi v3.

@Pawel-dev5
Copy link

I resolved that problem by sorting items with two parameters: title which is for me sorting items and you must add sorting by id, then pagination working great.

qs.stringify( { populate: { [${shopName}Prices]: { populate: '*', }, }, filters: { category: { $in: category, }, }, sort: ['title:asc', 'id:asc'], pagination: { page: paginationStart, pageSize: 50, }, }, { encodeValuesOnly: true, }, )

@LapX
Copy link

LapX commented Apr 12, 2023

This is a real problem. Why isn't this solved yet ?

@Bassel17
Copy link
Member

@LapX this is considered a rather complicated issue, fixing it correctly on the DB level would require a different solution for every database we support. We just didn't get the time or priority to move forward with it yet.

@philipp-sapronov
Copy link

philipp-sapronov commented May 10, 2023

Issue is still there, for example a Product which has many prices (relations) in different currencies

{ sort: { prices: { amount: 'asc' } }, filters: { prices: { currency: 'usd' } } }

It's returning duplicated products, looks like filters are ignored, is there a way to make something like post-filtering?

@iamandrewluca
Copy link
Contributor

We also started experiencing this issue 2 weeks ago.
Today we realised that the problem was in some custom code in the lifecycle hooks of the Content-Type.

So I would recommend giving a look at your lifecycle hooks if you have any.

@lcxfs1991
Copy link

Same issue here. Hope that it will be fixed soon.

@DeividVeloso
Copy link

Trying to sort using this code is not working and I could not find any documentation to it

Query:

image

I tried in many ways like

  "sort": [
   "name:desc"
  ]

  "sort": "name:desc"

@HarjasSodhi
Copy link

Issue is there when using repeatable components as well. Can anybody suggest a solution? I cant sort manually because i have applied pagination as well and getting all the data first and then sorting and paging it would not be efficient.

@MaximeDetaille
Copy link

Any update on this issue ?

@ihor-zinchenko
Copy link

same for me, any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug severity: high If it breaks the basic use of the product source: core:database Source is core/database package status: confirmed Confirmed by a Strapi Team member or multiple community members
Projects
Status: In progress
Status: No status
Status: Reproducible on v4