Skip to content

Issue/3460 #3463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,19 @@
*
* @author Gabriel Basilio
* @author Greg Turnquist
* @author Thorben Janssen
*/
class ProcedureParameter {

private final String name;
private final int position;
private final ParameterMode mode;
private final Class<?> type;

ProcedureParameter(@Nullable String name, ParameterMode mode, Class<?> type) {
ProcedureParameter(@Nullable String name, int position, ParameterMode mode, Class<?> type) {

this.name = name;
this.position = position;
this.mode = mode;
this.type = type;
}
Expand All @@ -46,6 +49,10 @@ public String getName() {
return name;
}

public int getPosition() {
return position;
}

public ParameterMode getMode() {
return mode;
}
Expand Down Expand Up @@ -76,6 +83,6 @@ public int hashCode() {

@Override
public String toString() {
return "ProcedureParameter{" + "name='" + name + '\'' + ", mode=" + mode + ", type=" + type + '}';
return "ProcedureParameter{" + "name='" + name + '\'' + ", position=" + position + ", mode=" + mode + ", type=" + type + '}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,18 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

import jakarta.persistence.NamedStoredProcedureQueries;
import jakarta.persistence.NamedStoredProcedureQuery;
import jakarta.persistence.ParameterMode;
import jakarta.persistence.StoredProcedureParameter;

import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;

import jakarta.persistence.NamedStoredProcedureQueries;
import jakarta.persistence.NamedStoredProcedureQuery;
import jakarta.persistence.ParameterMode;
import jakarta.persistence.StoredProcedureParameter;

/**
* A factory class for {@link StoredProcedureAttributes}.
*
Expand All @@ -44,6 +43,7 @@
* @author Jeff Sheets
* @author Gabriel Basilio
* @author Greg Turnquist
* @author Thorben Janssen
* @since 1.6
*/
enum StoredProcedureAttributeSource {
Expand Down Expand Up @@ -119,10 +119,7 @@ private StoredProcedureAttributes newProcedureAttributesFrom(Method method, Name
} else {

// try to discover the output parameter
outputParameters = extractOutputParametersFrom(namedStoredProc).stream() //
.map(namedParameter -> new ProcedureParameter(namedParameter.name(), namedParameter.mode(),
namedParameter.type())) //
.collect(Collectors.toList());
outputParameters = extractOutputParametersFrom(namedStoredProc);
}

return new StoredProcedureAttributes(namedStoredProc.name(), outputParameters, true);
Expand All @@ -137,33 +134,36 @@ private StoredProcedureAttributes newProcedureAttributesFrom(Method method, Name
*/
private ProcedureParameter createOutputProcedureParameterFrom(Method method, Procedure procedure) {

return new ProcedureParameter(procedure.outputParameterName(),
return new ProcedureParameter(procedure.outputParameterName(), 1,
procedure.refCursor() ? ParameterMode.REF_CURSOR : ParameterMode.OUT, method.getReturnType());
}

/**
* Translate all the {@Link NamedStoredProcedureQuery} parameters into a {@link List} of
* {@link StoredProcedureParameter}s.
* {@link ProcedureParameter}s.
*
* @param namedStoredProc
* @return
*/
private List<StoredProcedureParameter> extractOutputParametersFrom(NamedStoredProcedureQuery namedStoredProc) {
private List<ProcedureParameter> extractOutputParametersFrom(NamedStoredProcedureQuery namedStoredProc) {

List<StoredProcedureParameter> outputParameters = new ArrayList<>();
List<ProcedureParameter> outputParameters = new ArrayList<>();

int position = 1;
for (StoredProcedureParameter param : namedStoredProc.parameters()) {

switch (param.mode()) {
case OUT:
case INOUT:
case REF_CURSOR:
outputParameters.add(param);
outputParameters.add(new ProcedureParameter(param.name(), position, param.mode(), param.type()));
break;
case IN:
default:
continue;
// not an output parameter
}

position++;
}

return outputParameters;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* @author Jeff Sheets
* @author Jens Schauder
* @author Gabriel Basilio
* @author Thorben Janssen
* @since 1.6
*/
class StoredProcedureAttributes {
Expand Down Expand Up @@ -87,7 +88,7 @@ private List<ProcedureParameter> getParametersWithCompletedNames(List<ProcedureP

private ProcedureParameter getParameterWithCompletedName(ProcedureParameter parameter, int i) {

return new ProcedureParameter(completeOutputParameterName(i, parameter.getName()), parameter.getMode(),
return new ProcedureParameter(completeOutputParameterName(i, parameter.getName()), i+1, parameter.getMode(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like we could reuse parameter.getPosition() here instead of i+1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that should be better. I've added the change to this PR

parameter.getType());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
* @author Mark Paluch
* @author Jeff Sheets
* @author JyotirmoyVS
* @author Thorben Janssen
* @since 1.6
*/
class StoredProcedureJpaQuery extends AbstractJpaQuery {
Expand Down Expand Up @@ -119,15 +120,14 @@ Object extractOutputValue(StoredProcedureQuery storedProcedureQuery) {
List<ProcedureParameter> outputParameters = procedureAttributes.getOutputProcedureParameters();

if (outputParameters.size() == 1) {
return extractOutputParameterValue(outputParameters.get(0), 0, storedProcedureQuery);
return extractOutputParameterValue(outputParameters.get(0), storedProcedureQuery);
}

Map<String, Object> outputValues = new HashMap<>();

for (int i = 0; i < outputParameters.size(); i++) {
ProcedureParameter outputParameter = outputParameters.get(i);
outputValues.put(outputParameter.getName(),
extractOutputParameterValue(outputParameter, i, storedProcedureQuery));
for (ProcedureParameter outputParameter : outputParameters) {
outputValues.put(!outputParameter.getName().isEmpty() ? outputParameter.getName() : outputParameter.getPosition()+"",
extractOutputParameterValue(outputParameter, storedProcedureQuery));
}

return outputValues;
Expand All @@ -136,14 +136,12 @@ Object extractOutputValue(StoredProcedureQuery storedProcedureQuery) {
/**
* @return The value of an output parameter either by name or by index.
*/
private Object extractOutputParameterValue(ProcedureParameter outputParameter, Integer index,
private Object extractOutputParameterValue(ProcedureParameter outputParameter,
StoredProcedureQuery storedProcedureQuery) {

JpaParameters methodParameters = getQueryMethod().getParameters();

return useNamedParameters && StringUtils.hasText(outputParameter.getName())
? storedProcedureQuery.getOutputParameterValue(outputParameter.getName())
: storedProcedureQuery.getOutputParameterValue(methodParameters.getNumberOfParameters() + index + 1);
: storedProcedureQuery.getOutputParameterValue(outputParameter.getPosition());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,11 @@

package org.springframework.data.jpa.repository.procedures;

import static org.assertj.core.api.Assertions.*;

import jakarta.persistence.Entity;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.NamedStoredProcedureQuery;
import jakarta.persistence.ParameterMode;
import jakarta.persistence.StoredProcedureParameter;
import static org.assertj.core.api.Assertions.assertThat;

import java.math.BigDecimal;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;

Expand All @@ -45,7 +38,6 @@
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.data.jpa.repository.query.Procedure;
import org.springframework.data.jpa.util.DisabledOnHibernate61;
import org.springframework.data.jpa.util.DisabledOnHibernate62;
import org.springframework.jdbc.datasource.init.DataSourceInitializer;
import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator;
Expand All @@ -60,14 +52,22 @@
import org.springframework.transaction.annotation.Transactional;
import org.testcontainers.containers.PostgreSQLContainer;

import jakarta.persistence.Entity;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.NamedStoredProcedureQuery;
import jakarta.persistence.ParameterMode;
import jakarta.persistence.StoredProcedureParameter;

/**
* Testcase to verify {@link org.springframework.jdbc.object.StoredProcedure}s work with Postgres.
*
* @author Gabriel Basilio
* @author Greg Turnquist
* @author Yanming Zhou
* @author Thorben Janssen
*/
@DisabledOnHibernate61 // GH-2903
@Transactional
@ExtendWith(SpringExtension.class)
@ContextConfiguration(classes = PostgresStoredProcedureIntegrationTests.Config.class)
Expand Down Expand Up @@ -150,12 +150,28 @@ void testEntityListFromNamedProcedure() {
new Employee(4, "Gabriel"));
}

@Test // 3460
void testPositionalInOutParameter() {

Map results = repository.positionalInOut(1, 2);

assertThat(results.get("2")).isEqualTo(2);
assertThat(results.get("3")).isEqualTo(3);
}

@Entity
@NamedStoredProcedureQuery( //
name = "get_employees_postgres", //
procedureName = "get_employees", //
parameters = { @StoredProcedureParameter(mode = ParameterMode.REF_CURSOR, type = void.class) }, //
resultClasses = Employee.class)
@NamedStoredProcedureQuery( //
name = "positional_inout", //
procedureName = "positional_inout_parameter_issue3460", //
parameters = { @StoredProcedureParameter(mode = ParameterMode.IN, type = Integer.class),
@StoredProcedureParameter(mode = ParameterMode.INOUT, type = Integer.class),
@StoredProcedureParameter(mode = ParameterMode.OUT, type = Integer.class) }, //
resultClasses = Employee.class)
public static class Employee {

@Id
Expand Down Expand Up @@ -234,6 +250,9 @@ public interface EmployeeRepositoryWithRefCursor extends JpaRepository<Employee,

@Procedure(name = "get_employees_postgres", refCursor = true)
List<Employee> entityListFromNamedProcedure();

@Procedure(name = "positional_inout")
Map positionalInOut(Integer in, Integer inout);
}

@EnableJpaRepositories(considerNestedRepositories = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@
*
* @author Oliver Gierke
* @author Jens Schauder
* @author Thorben Janssen
*/
class StoredProcedureAttributesUnitTests {

@Test // DATAJPA-681
void usesSyntheticOutputParameterNameForAdhocProcedureWithoutOutputName() {

StoredProcedureAttributes attributes = new StoredProcedureAttributes("procedure",
new ProcedureParameter(null, ParameterMode.OUT, Long.class));
new ProcedureParameter(null, 1, ParameterMode.OUT, Long.class));
assertThat(attributes.getOutputProcedureParameters().get(0).getName()).isEqualTo(SYNTHETIC_OUTPUT_PARAMETER_NAME);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,21 @@ BEGIN
OPEN ref FOR SELECT * FROM employee WHERE employee.ID = 3;
END;
$BODY$;;

CREATE OR REPLACE PROCEDURE get_employees_count(OUT results integer)
LANGUAGE 'plpgsql'
AS
$BODY$
BEGIN
results = (SELECT COUNT(*) FROM employee);
END;
$BODY$;;

CREATE OR REPLACE PROCEDURE positional_inout_parameter_issue3460(IN inParam integer, INOUT inoutParam integer, OUT outParam integer)
LANGUAGE 'plpgsql'
AS
$BODY$
BEGIN
outParam = 3;
END;
$BODY$;;
Loading