jxls icon indicating copy to clipboard operation
jxls copied to clipboard

Set fetch size for JdbcHelper

Open fabianMendez opened this issue 4 years ago • 4 comments

Hi there,

I was testing this library to generate a quite large report with over ~150k rows from an SQL database, one of the issues I found was that it was taking so much time (~3 minutes) and it was due to the rather low value of rows fetched per network call, ie: if no value is specified using the setFetchSize method of the statement, it fetches 10 rows at a time by default, which means over 15k network calls to the database.

Once I overwrote the query method from the JdbcHelper class and specified 1000 rows as the fetch size, the same report took only ~25s.

So I am proposing to create a new constructor for the JdbcHelper class which allows to specify the fetchSize to use. The implementation is pretty simple so I can create the PR, but I am not sure if this can be tested because the the in-memory database probably won't be affected by a network optimization.

fabianMendez avatar Oct 20 '20 14:10 fabianMendez

It is already possible to specify the fetch size for the connection as a connection property.

Oracle=defaultRowPrefetch, PostgreSQL=defaultRowFetchSize, MySQL/MariaDB=defaultFetchSize, DB2=fetchSize

@fabianMendez & @leonate - What is your opinion?

SoltauFintel avatar Oct 21 '20 08:10 SoltauFintel

That would be an option, however, I am not sure if it is possible to specify such properties without using the JDBC URL, because that would affect the whole application using the connection.

According to this, it is possible but is driver specific, so one would have to check if the connection is an instance of a class to use the specific method for that class.

In short, I think it would be a lot of less hassle if it the JdbcHelper class supports it as an optional parameter of the constructor.

fabianMendez avatar Oct 21 '20 17:10 fabianMendez

@leonate - what's your opinion?

SoltauFintel avatar Oct 27 '20 08:10 SoltauFintel

An option to set the fetch size makes sense to me. Need to take a look what changes would be needed to support it.

leonate avatar Feb 21 '21 16:02 leonate