darrachequesne / spring-data-jpa-datatables

Spring Data JPA extension to work with the great jQuery plugin DataTables (https://datatables.net/)
Apache License 2.0
440 stars 174 forks source link

[question] specifications and counting total records #153

Closed chpasha closed 1 year ago

chpasha commented 1 year ago

Hi there, I don't quite get the logic behind existence of two specification params and counting of total records. Let's take a look at the longest findAll Method in Repo

findAll(DataTablesInput input,
      Specification<T> additionalSpecification, Specification<T> preFilteringSpecification,
      Function<T, R> converter)

it takes two specifications, where the shorter versions of the method takes only additionalSpecification as param and sets preFilteringSpecification specification to null

 public DataTablesOutput<T> findAll(DataTablesInput input,
      Specification<T> additionalSpecification) {
    return findAll(input, additionalSpecification, null, null);
  }

But...it is the preFilteringSpecification that affects the count of the total records (which is important for pagination) long recordsTotal = preFilteringSpecification == null ? count() : count(preFilteringSpecification);

so, if we use the shorter version of the method with additionalSpecification only, it can contain any conditions reducing the number of rows found, but the repo will still count them as select * from table. So, unless I totally misunderstood the concept behind the current API, a better approach (in my opinion) would be either to have only one spec param or (if we want to make a count query more efficient by not joining something, that doesn't affect the number of rows) allow two specs, but let the preFilteringSpecification to be the default one and call the other accordingly, like countSpecification, so that API user understands what the each spec is meant for? I use the library on very rare occasions (actually the second time in my life), so excuse me, if I've totally missed something :-)

darrachequesne commented 1 year ago

But...it is the preFilteringSpecification that affects the count of the total records (which is important for pagination)

The preFilteringSpecification is indeed used to compute the recordsTotal field, the Y value at the bottom of the table:

Showing 1 to 10 of X entries (filtered from Y total entries)

Both the additionalSpecification and preFilteringSpecification are then used to compute the recordsFiltered field (the X value).

For example, one can use:

Regarding which one should come first, that's a great question! Historically, the additionalSpecification argument was implemented first, and the preFilteringSpecification was added later, hence the current API.

chpasha commented 1 year ago

I see, thanks. To tell the truth, I don't fully understand the use case where we hide records but still show totals for all - that would break the pagination, wouldn't it? We need to know the real totals to calculate a number of pages. If we have page size 10, 11 records in total and 1 record hidden, we would see two pages with 10 records on the first page and the second page would lead to empty result, right?

darrachequesne commented 1 year ago

To tell the truth, I don't fully understand the use case where we hide records but still show totals for all

That's not the case. Let's say you have 100 entities in your table (SELECT count(1) FROM mytable).

If you provide a preFilteringSpecification that hides 20 entities, and then an additionalSpecification that filters 30 entities out, you will see:

Showing 1 to 10 of 50 entries (filtered from 80 total entries)

Hope that's clearer now!

chpasha commented 1 year ago

Yes, I understand now, thank you. I had to look into the source code (could do that before asking :-) ) - there is another value "recordsFiltered" that allows the paging to work properly