Keyset-scrolling queries add identifier columns twice when Sort already sorts by Id

[sergey-morenets]

Hi

I have a repository with the following method:

	Window<Order> findAllBy(ScrollPosition position, 
			Sort sort);

I tried to use keyset-scrolling and specified createdAt and id as keyset properties. Also I added "createdAt" to sort properties:

		orders = orderRepository.findAllBy(
				ScrollPosition.forward(Map.of("id", id,
						"createdAt", LocalDateTime.now())), 
					Sort.by("createdAt"));

The resulting SQL query uses following sort order: firstly createdAt, then id:

from ORDERS o1_0 where o1_0.createdAt>? or o1_0.createdAt=? and o1_0.id>? order by o1_0.createdAt,o1_0.id

But I need the opposite order (id and then createdAt). So I added "id" property to the Sort.by construction:

		orders = orderRepository.findAllBy(
				ScrollPosition.forward(Map.of("id", id,
						"createdAt", LocalDateTime.now())), 
					Sort.by("id", "createdAt"));

However the resulting SQL query contains ORDER BY clause where "id" column is specified twice:

from ORDERS o1_0 where o1_0.id>? or o1_0.id=? and o1_0.createdAt>? or o1_0.id=? and o1_0.createdAt=? and o1_0.id>? order by o1_0.id,o1_0.createdAt,o1_0.id

[sergey-morenets]

Similar issue occurs if I specified ordering in the method name:

Window<Order> findAllByOrderByIdAscCreatedAt(ScrollPosition position);

If I call this method:

		orders = orderRepository.findAllByOrderByIdAscCreatedAt(
				ScrollPosition.forward(Map.of("id", id,
						"createdAt", LocalDateTime.now())));

Then resulting SQL query contains two mistakes:

from ORDERS o1_0 where o1_0.id>? or o1_0.id=? and o1_0.createdAt>? or o1_0.id=? and o1_0.createdAt=? and o1_0.id>? order by o1_0.id,o1_0.createdAt,o1_0.id

1. Multiple o1_0.id=? and o1_0.id>? parts.
2. Duplicate fragment o1_0.id in ORDER BY clause.

[quaff]

spring-data-jpa/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java

Lines 60 to 72 in 8cae8cd

	public static Sort createSort(KeysetScrollPosition position, Sort sort, JpaEntityInformation<?, ?> entity) {
	KeysetScrollDelegate delegate = KeysetScrollDelegate.of(position.getDirection());
	Sort sortToUse;
	if (entity.hasCompositeId()) {
	sortToUse = sort.and(Sort.by(entity.getIdAttributeNames().toArray(new String[0])));
	} else {
	sortToUse = sort.and(Sort.by(entity.getRequiredIdAttribute().getName()));
	}
	return delegate.getSortOrders(sortToUse);
	}

Id shouldn't be added to sort if user provide explicit one, WDYT? @mp911de

[mp911de]

You're right, we should not add identifiers to Sort if Sort contains them.

[quaff]

@mp911de It seems that id property will always be used for keyset scrolling, what if id property is not applicable for sort and we could scroll base on another unique property?
For example we have an UUID as id property and an Long as biz_sequence property which is unique and monotone increasing, then this biz_sequence is perfect for keyset scrolling, and id property should not participate in.

[mp911de]

Please file a new ticket to discuss this matter. We require a property to make the database rows uniquely identifiable to avoid skipping rows with the same sort value as their previous row.

[quaff]

Please file a new ticket to discuss this matter. We require a property to make the database rows uniquely identifiable to avoid skipping rows with the same sort value as their previous row.

I've created #3013, should I file another ticket?

[mp911de]

That one is a pull request already. We can continue the discussion there.