search

nextcloud / user_sql

🔒 App for authenticating Nextcloud users using SQL
GNU Affero General Public License v3.0
66 stars 33 forks source link

Selecting and counting users from the User-Group table needs DISTINCT… #172

Closed rotdrop closed 10 months ago

rotdrop commented 2 years ago

… to avoid duplicates.

In particular when using the catch-all group ("Default Group" setting) the queries which count and select users from the User-Group table need the "DISTINCT" option: the catch-all group is replaced by a '%' wild-card in the query. As users may belong by design to more than one group counting and selecting users comes out wrong. Even worse: the many duplicates interfere with the paging logic of the user admin-settings.

Signed-off-by: Claus-Justus Heine himself@claus-justus-heine.de

d-jsb commented 2 years ago

Tried that patch manually to see if it would solve another issue. Turns out that WITH this patch the number of group members is NOT displayed correctly anymore but 1 is wrongly shown so I reverted the change.

rotdrop commented 2 years ago

Tried that patch manually to see if it would solve another issue. Turns out that WITH this patch the number of group members is NOT displayed correctly anymore but 1 is wrongly shown so I reverted the change.

Are you sure? Looking at my patch I would say that this cannot be. At least I do not understand how the DISTINCT (which is the only added change) could lead to this. Are you sure? Can you please explain how the "DISTINCT" could lead to wrong behaviour? The DISTINCT is in front of the selected uid so it should really just avoid counting uids twice???? I am confused.

d-jsb commented 2 years ago

Tried that patch manually to see if it would solve another issue. Turns out that WITH this patch the number of group members is NOT displayed correctly anymore but 1 is wrongly shown so I reverted the change.

Are you sure? Looking at my patch I would say that this cannot be. At least I do not understand how the DISTINCT (which is the only added change) could lead to this. Are you sure? Can you please explain how the "DISTINCT" could lead to wrong behaviour? The DISTINCT is in front of the selected uid so it should really just avoid counting uids twice???? I am confused.

So am I, because from my head I agree with what you say but the little bubbles showed the correct number of group members before applying that change and after apply showed one. On the other hand, something is broken anyhow presumably because I am on NC 24. Getting user data randomly works or does not. Log shows memory exhaustion even after raising php memory limit

mlojewski-me commented 10 months ago

https://github.com/nextcloud/user_sql/pull/190 already has these changes