I found one problem with users Indexing. Not all users are indexed. Probably this bug related to PostgreSQL. I found problem with "range" method in select to DB. This select get the same results for different pager number.

Comments

Anton4yk created an issue. See original summary.

anton4uk’s picture

Status: Needs work » Needs review
StatusFileSize
new714 bytes
borisson_’s picture

Priority: Critical » Major
Status: Needs review » Needs work
Issue tags: -indexing +Needs tests

I have no idea why this would make a difference or what's actually going on here.

So we should write a test that describes the problem here.

PS: It seems you (like many others – it's really easy to misinterpret) are confused by the "Issue tags" field. As the guidelines state, they aren't meant for free text tags related to the issue, but only for specific categorization purposes, usually by module maintainers.
So, if you aren't sure your current usage is correct, please just leave the field empty.

drunken monkey’s picture

Thanks a lot for reporting this and already providing a fix!

Joris: Admittedly, the issue description isn't very helpful, but the patch points clearly to the problem: if we don't specify an explicit sort, the order of returned result rows is undefined. Therefore, if we don't request all results at once but use paging, it's not guaranteed that separate pages will actually contain different results. (Especially if paging through all results takes some time, which means new rows might be added in the meantime.)
MySQL generally sorts by the time a row was added, as far as I know, so this problem doesn't exist there and apparently we just relied on that. It's, however, completely possible that for other DBMSs (or maybe even special MySQL setups or scenarios) this will fail, returning some item IDs multiple times and others not at all. (From the issue summary it sounds like Postgres might even have a check for that: if someone requests a range but specifies no sort, just return the first $limit results regarding of $offset – serves them right, the dirty, MySQL-loving bugger!)

So, adding a sort there is definitely the clean thing to do.

Regarding tests, it turns out (as far as I can see) that that method was always just tested implicitly, but never explicitly, even though it's very easy to test. I've now added some tests, and actually discovered a different bug in the process, or even multiple ones.
Patch attached, with two different "test only" variants so we can also see whether we can reproduce the original problem. (Might not be the case, though – as said, it might depend on the specific DBMS setup.)

drunken monkey’s picture

Title: Indexing of users works incorrectly. » Fix bugs in entity datasource's item discovery code in edge cases
Component: General code » Plugins
Priority: Major » Normal

As feared, the patch without the added sort doesn't actually fail. But, it also definitely doesn't break anything, shouldn't matter much performance-wise (I hope) and, as explained, I'm pretty sure it's the correct way to do this, even if it also works without the sort in most cases.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

Sure, now that you explain it, I understand the problem, fix looks good and the added testcode looks solid.

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

OK, good to hear. Thanks for your feedback!
Committed.
Thanks again also to Anton4yk for the initial patch!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.