nestedj icon indicating copy to clipboard operation
nestedj copied to clipboard

NestedNodeRebuildingQueryDelegate findFirst method implementation actually returns the "last"

Open kousalik opened this issue 4 years ago • 1 comments

Hi, If I understand this correctly, all the the implementations are wrong and actually returning the last root (order by id DESC) - unlike what the method suggests.

I believe the tests (like testGetPrevSiblingRoot) pass only thanks to the behaviour of the SEQ sequence. The Y node inserted in the test gets saved with ID = 1 (first time the sequesnce is used) and the rest of the test data is behind it (ID's 1000, 2000 etc ...)

These two things combined make the test pass. I don't think this breaks anything. Mostly just makes the code tricky to understand.

We found this when implementing Mybatis version for SQL server .. where we used IDENTITY id (autoincrement) which updates the ID sequence as the test data gets inserted .. so the node Y gets ID = 16001.

EDIT: I understand that it depends on the definition of "first" but the assumption based on the growing sequence of IDs is that tha last is the node with the highest ID.

Happy to prepare PR if you agree.

kousalik avatar Jan 29 '22 21:01 kousalik

Thanks for reporting this, I'll take a look in the coming days and come up with a fix.

eXsio avatar Jan 30 '22 13:01 eXsio