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

limit isnt respected when max_depth is used on comments endpoint #3065

Closed
4 tasks done
markcellus opened this issue Jun 13, 2023 · 5 comments
Closed
4 tasks done

limit isnt respected when max_depth is used on comments endpoint #3065

markcellus opened this issue Jun 13, 2023 · 5 comments
Labels
area: api bug Something isn't working

Comments

@markcellus
Copy link

markcellus commented Jun 13, 2023

  • Did you check to see if this issue already exists?
  • Is this only a single bug? Do not put multiple bugs in one issue.
  • Is this a question or discussion? Don't use this, use https://lemmy.ml/c/lemmy_support .
  • Is this a UI / front end issue? Use the lemmy-ui repo.

Issue Summary

The getComments endpoint doesn't seem to respect the limit when used with max_depth

Steps to Reproduce

  1. Navigate to https://feddit.de/api/v3/comment/list?post_id=745959&type_=All&max_depth=4&limit=1 or curl or whatever
  2. Notice that it returns way more comments than just the 1 I've set in the limit param

Technical details

  • Please post your log: sudo docker-compose logs > lemmy_log.out.
  • What OS are you trying to install lemmy on?
  • Any browser console errors?
@markcellus markcellus added the bug Something isn't working label Jun 13, 2023
@kartikynwa
Copy link
Contributor

For now this is intended, not sure what the long term resolution is:

// TODO limit question. Limiting does not work for comment threads ATM, only max_depth
// For now, don't do any limiting for tree fetches
// https://stackoverflow.com/questions/72983614/postgres-ltree-how-to-limit-the-max-number-of-children-at-any-given-level
// Don't use the regular error-checking one, many more comments must ofter be fetched.
// This does not work for comment trees, and the limit should be manually set to a high number
//
// If a max depth is given, then you know its a tree fetch, and limits should be ignored

@Nutomic
Copy link
Member

Nutomic commented Mar 15, 2024

Closing in favor of #4543

@Nutomic Nutomic closed this as completed Mar 15, 2024
@markcellus
Copy link
Author

markcellus commented Mar 15, 2024

Thanks for working on this. I took a look at #4543 issue and doesnt seem related. Was closing this issue and marking it completed a mistake? 🤔

@Nutomic
Copy link
Member

Nutomic commented Mar 18, 2024

I believe its part of the same problem which is described in this comment. cc @dessalines

@dessalines
Copy link
Member

dessalines commented Mar 25, 2024

Yes that's correct, the new issue is used to track this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants