migrate naming / terminology away from "keyset"

[gavinking]

See #504.

WDYT, folks?

[gavinking]

Note that I've split this into 3 commits, with changes to javadoc, spec, and APIs done separately. Hope that makes for easier review.

[njr-11]

This looks pretty good. I think the term "cursor-based pagination" will be better understood than "key-based pagination" and it better fits with the name CursoredPage. I added suggestions to change it, and after getting a ways into it was amazed at how many times we refer to it in the spec. I also wonder if "key values" will be misinterpreted as having multiple keys, rather than multiple elements that make up a single key. I added comments in some places for that as well.

[gavinking]

I think the term "cursor-based pagination" will be better understood than "key-based pagination"

Yeah so I started typing that but then wondered if it was too easily-misunderstood as something to do with database cursors which it really most definitely is not.

[njr-11]

I think the term "cursor-based pagination" will be better understood than "key-based pagination"

Yeah so I started typing that but then wondered if it was too easily-misunderstood as something to do with database cursors which it really most definitely is not.

Both "cursor" and "key" have other meanings in the database and are overloaded in Jakarta Data. Our options will be that we can either use one of these overloaded terms or we can come up with a unique term for the idea of a composite key that is made up of the entity attribute values from the sort criteria. One attempt at the latter was "keyset", which is an already-existing term and not something we invented for Jakarta Data. A big problem with it is that its name will be literally interpreted as a "set of keys", but it is neither a set nor do the values that form it need to be keys. I do think it makes sense to try to find a better term.

I don't see using overloaded terms to necessarily be a problem. Jakarta Data defines a CursoredPage and a PageRequest.Cursor that matches up nicely with "Cursor-based Pagination" and gets the point across that a page is computed relative to a particular query result. "Key-based Pagination" doesn't have such direct correspondence to the API and will likely get users thinking of the primary key / @Id entity attribute. It is true that you could perform this sort of pagination based solely on the primary key, but I would expect it to typically be used in combination with additional sort criteria that are more meaningful to the user. I think most people are going to think of key as singular, with the possibility of composite key being an afterthought, which will make the term "Key-based Pagination" less intuitive than "Cursor-based Pagination".

[gavinking]

OK I pushed a commit which makes the global change to "cursor-based".

[njr-11]

The latest set of updates looks good. It looks like we lost a few other review comments which happened to overlap the same areas, and I did my best to re-enter them. This is mostly around avoidance of the phrase reverse direction which can mislead users into believing their previous pages will be sorted in reverse. There were also a few places with keyset that were missed and a few review comments to avoid confusion over "key values" being misunderstood as values that are keys rather than values for a single composite key. There should be suggestions for everything remaining in the pull now.

[gavinking]

I committed the suggestions.

[njr-11]

I committed the suggestions.

Thanks. I still see 5 remaining, all of which are related to clarifying that the user is not being asked to supply multiple key values. They are being asked to supply multiple values that together form a single composite key.
At this point, it should be fine to approve it in advance because these are minor clarifications.

[njr-11]

I just spotted two others that were lost and re-added them.

[gavinking]

I still see 5 remaining

I have committed the ones I can find, but I dunno, this system seems to be a bit flakey.

[gavinking]

I managed to find some more by laboriously scrolling through the diff. They weren't showing up here.

[njr-11]

I managed to find some more by laboriously scrolling through the diff. They weren't showing up here.

Yeah, they were buried under git's "Show hidden comments" section, and even then were intermixed with a bunch of outdated ones. I went through and resolved all of the outdated ones and only after that was it clear which were remaining. It looks like you got them all now.

@otaviojava did you have any comments on this one before merging it? Our deadline for changes unrelated to the Query Language is this Friday.

[njr-11]

I added a commit to resolve merge conflicts with the {@code} pull

[gavinking]

Going to have to squash this because the conflicts are bad.

[gavinking]

I added a commit to resolve merge conflicts with the {@code} pull

Oh you did it? OK.

[gavinking]

IMO, this one should now be merged.

[otaviojava]

I managed to find some more by laboriously scrolling through the diff. They weren't showing up here.

Yeah, they were buried under git's "Show hidden comments" section, and even then were intermixed with a bunch of outdated ones. I went through and resolved all of the outdated ones and only after that was it clear which were remaining. It looks like you got them all now.

@otaviojava did you have any comments on this one before merging it? Our deadline for changes unrelated to the Query Language is this Friday.

Nope, let's merge it.