From 781faf1306fe4b026dec4c3e3d21e4c0494b2c0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henning=20P=C3=B6ttker?= Date: Sat, 3 Jul 2021 00:37:25 +0200 Subject: [PATCH] Fix multi-threaded empty read for JdbcPagingItemReader On empty input, the JdbcPagingItemReader cannot derive a start value for the sort key to be used in further queries. For multi-threaded steps, it is thus necessary to prevent the reader from trying to read further pages if the first page is empty. Issue #3898 --- .../item/database/JdbcPagingItemReader.java | 13 ++-- .../JdbcPagingItemReaderAsyncTests.java | 5 +- ...bcPagingItemReaderEmptyResultSetTests.java | 68 +++++++++++++++++++ 3 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java index 889cdcd7b..dfab1de12 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/database/JdbcPagingItemReader.java @@ -19,7 +19,7 @@ package org.springframework.batch.item.database; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; -import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -179,7 +179,6 @@ public class JdbcPagingItemReader extends AbstractPagingItemReader impleme } @Override - @SuppressWarnings("unchecked") protected void doReadPage() { if (results == null) { results = new CopyOnWriteArrayList<>(); @@ -190,7 +189,7 @@ public class JdbcPagingItemReader extends AbstractPagingItemReader impleme PagingRowMapper rowCallback = new PagingRowMapper(); - List query; + List query; if (getPage() == 0) { if (logger.isDebugEnabled()) { @@ -211,7 +210,7 @@ public class JdbcPagingItemReader extends AbstractPagingItemReader impleme } } - else { + else if (startAfterValues != null) { previousStartAfterValues = startAfterValues; if (logger.isDebugEnabled()) { logger.debug("SQL used for reading remaining pages: [" + remainingPagesSql + "]"); @@ -225,9 +224,11 @@ public class JdbcPagingItemReader extends AbstractPagingItemReader impleme getParameterList(parameterValues, startAfterValues).toArray(), rowCallback); } } + else { + query = Collections.emptyList(); + } - Collection result = (Collection) query; - results.addAll(result); + results.addAll(query); } @Override diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java index 9113c20dc..607178a79 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderAsyncTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2014 the original author or authors. + * Copyright 2009-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -80,7 +80,8 @@ public class JdbcPagingItemReaderAsyncTests { @Before public void init() { JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); - maxId = jdbcTemplate.queryForObject("SELECT MAX(ID) from T_FOOS", Integer.class); + Integer maxIdResult = jdbcTemplate.queryForObject("SELECT MAX(ID) from T_FOOS", Integer.class); + maxId = maxIdResult == null ? 0 : maxIdResult; for (int i = maxId + 1; i <= ITEM_COUNT; i++) { jdbcTemplate.update("INSERT into T_FOOS (ID,NAME,VALUE) values (?, ?, ?)", i, "foo" + i, i); } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java new file mode 100644 index 000000000..ee821ad3f --- /dev/null +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/database/JdbcPagingItemReaderEmptyResultSetTests.java @@ -0,0 +1,68 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.item.database; + +import static org.junit.Assert.assertNull; + +import java.util.Collections; + +import javax.sql.DataSource; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.batch.item.ItemReader; +import org.springframework.batch.item.database.support.HsqlPagingQueryProvider; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jdbc.core.SingleColumnRowMapper; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration(locations = "JdbcPagingItemReaderCommonTests-context.xml") +public class JdbcPagingItemReaderEmptyResultSetTests { + + private static final int PAGE_SIZE = 2; + private static final int EMPTY_READS = PAGE_SIZE + 1; + + @Autowired + private DataSource dataSource; + + @Test + public void testMultiplePageReadsOnEmptyResultSet() throws Exception { + final ItemReader reader = getItemReader(); + for (int i = 0; i < EMPTY_READS; i++) { + assertNull(reader.read()); + } + } + + private ItemReader getItemReader() throws Exception { + JdbcPagingItemReader reader = new JdbcPagingItemReader<>(); + reader.setDataSource(dataSource); + HsqlPagingQueryProvider queryProvider = new HsqlPagingQueryProvider(); + queryProvider.setSelectClause("select ID"); + queryProvider.setFromClause("from T_FOOS"); + queryProvider.setWhereClause("1 = 0"); + queryProvider.setSortKeys(Collections.singletonMap("ID", Order.ASCENDING)); + reader.setQueryProvider(queryProvider); + reader.setRowMapper(new SingleColumnRowMapper<>()); + reader.setPageSize(PAGE_SIZE); + reader.afterPropertiesSet(); + reader.setSaveState(false); + + return reader; + } +}