Skip to content

UDP Implementation #86

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.

Sign up for GitHub

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 1 commit into from
Closed

UDP Implementation #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/main/java/module-info.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@
requires transitive r2dbc.spi;

exports oracle.r2dbc;
exports oracle.r2dbc.impl;
Copy link
Member

Choose a reason for hiding this comment

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

The impl package is not part of the public API. We can not export it.

}
3 changes: 2 additions & 1 deletion src/main/java/oracle/r2dbc/impl/OracleReadableImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ else if (Object.class.equals(type)) {
// Use the default type mapping if Object.class has been specified.
// This method is invoked recursively with the default mapping, so long
// as Object.class is not also the default mapping.
Class<?> defaultType = readablesMetadata.get(index).getJavaType();
Class<?> defaultType = readablesMetadata.get(index).getType().getJavaType();
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why we need this change? I feel like I'm missing something :)


value = Object.class.equals(defaultType)
? jdbcReadable.getObject(index, Object.class)
: convert(index, defaultType);
Expand Down
47 changes: 35 additions & 12 deletions src/main/java/oracle/r2dbc/impl/OracleStatementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ private void bindObject(int index, Object object) {
if (object == null){
bindValues[index] = NULL_BIND;
}
else if (object instanceof Parameter) {
else if (object instanceof Parameter || object instanceof UserDefinedType) {
Copy link
Member

Choose a reason for hiding this comment

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

The R2DBC SPI uses instances of Parameter.Out to represent out binds. It should be possible to do so with UserDefinedType, as in:

statement.bind(Parameters.out(new UserDefinedType(...)))

There should be no need to support mapping UserDefinedType objects directly.

bindParameter(index, (Parameter)object);
}
else if (object instanceof Parameter.In
Expand All @@ -716,7 +716,7 @@ else if (object instanceof Parameter.In
* @throws IllegalArgumentException If the Java or SQL type of the
* {@code parameter} is not supported.
*/
private void bindParameter(int index, Parameter parameter) {
private void bindParameter(int index, Object parameter) {

if (parameter instanceof Parameter.Out) {
if (! batch.isEmpty())
Expand All @@ -725,21 +725,35 @@ private void bindParameter(int index, Parameter parameter) {
throw outParameterWithGeneratedValues();
}

Type parameterType;
Object parameterValue;
if(parameter instanceof Parameter){
parameterType= ((Parameter) parameter).getType();
parameterValue= ((Parameter) parameter).getValue();
} else {
throw new IllegalArgumentException("Invalid parameter type: "+parameter);
}

// TODO: This method should check if Java type can be converted to the
// specified SQL type. If the conversion is unsupported, then JDBC
// setObject(...) will throw when this statement is executed. The correct
// behavior is to throw IllegalArgumentException here, and not from
// execute()
Type r2dbcType =
requireNonNull(parameter.getType(), "Parameter type is null");
SQLType jdbcType = toJdbcType(r2dbcType);
Type r2dbcType= requireNonNull(parameterType, "Parameter type is null");

SQLType jdbcType;
if(r2dbcType instanceof UserDefinedType){
jdbcType = toJdbcType(r2dbcType.getJavaType());
} else {
jdbcType = toJdbcType(r2dbcType);
}

if (jdbcType == null) {
throw new IllegalArgumentException(
"Unsupported SQL type: " + r2dbcType);
}

requireSupportedJavaType(parameter.getValue());
requireSupportedJavaType(parameterValue);
bindValues[index] = parameter;
}

Expand Down Expand Up @@ -983,7 +997,7 @@ protected final Publisher<Void> bind(Object[] binds) {
List<Publisher<Void>> bindPublishers = null;
for (int i = 0; i < binds.length; i++) {

if (binds[i] instanceof Parameter.Out
if ((((binds[i] instanceof Parameter.Out) || (binds[i] instanceof UserDefinedType)))
&& !(binds[i] instanceof Parameter.In))
continue;

Expand Down Expand Up @@ -1339,7 +1353,7 @@ private JdbcCall(
super(callableStatement, bindValues);

outBindIndexes = IntStream.range(0, bindValues.length)
.filter(i -> bindValues[i] instanceof Parameter.Out)
.filter(i -> bindValues[i] instanceof Parameter.Out || bindValues[i] instanceof UserDefinedType)
.toArray();

OutParameterMetadata[] metadataArray =
Expand All @@ -1351,8 +1365,10 @@ private JdbcCall(
// Use the parameter name, or the index if the parameter is unnamed
String name = requireNonNullElse(
parameterNames.get(bindIndex), String.valueOf(i));
metadataArray[i] = createParameterMetadata(
name, ((Parameter)bindValues[bindIndex]).getType());
if(bindValues[bindIndex] instanceof Parameter){
metadataArray[i] = createParameterMetadata(
name, ((Parameter)bindValues[bindIndex]).getType());
}
}

this.metadata = createOutParametersMetadata(metadataArray);
Expand All @@ -1377,8 +1393,15 @@ private Publisher<Void> registerOutParameters() {

for (int i : outBindIndexes) {
Type type = ((Parameter) binds[i]).getType();
SQLType jdbcType = toJdbcType(type);
callableStatement.registerOutParameter(i + 1, jdbcType);
SQLType jdbcType;

if(type instanceof UserDefinedType){
jdbcType = toJdbcType(type.getJavaType());
callableStatement.registerOutParameter(i+1,jdbcType,type.getName());
} else {
jdbcType = toJdbcType(type);
callableStatement.registerOutParameter(i+1, jdbcType);
}
}
});
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/oracle/r2dbc/impl/SqlTypeMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ static Type toR2dbcType(int jdbcTypeNumber) {
* @return A JDBC SQL type
*/
static SQLType toJdbcType(Type r2dbcType) {
if(r2dbcType instanceof UserDefinedType){
return JAVA_TO_SQL_TYPE_MAP.get(r2dbcType.getClass());
}
return r2dbcType instanceof Type.InferredType
? toJdbcType(r2dbcType.getJavaType())
: R2DBC_TO_JDBC_TYPE_MAP.get(r2dbcType);
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/oracle/r2dbc/impl/UserDefinedType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package oracle.r2dbc.impl;
Copy link
Member

Choose a reason for hiding this comment

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

I think the need for exporting the impl package came from here.
On the Issue thread, I had suggested a factory method on OracleR2dbcTypes. We can make UserDefinedType an inner class of OracleR2dbcTypes, also private and static. We can then have the factory method return an instance of this class. This would eliminate the need to export the impl package.
A factory method will also allow us to return an instance of the Type interface, rather than a specific concrete class. By returning an interface, we will have more freedom to change the code later without breaking existing code that calls the method.


import io.r2dbc.spi.Type;

public final class UserDefinedType implements Type {
Copy link
Member

Choose a reason for hiding this comment

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

We must write a JavaDoc before merging any code. The doc should capture the rationale which was present in the mind of the programmer who wrote the code: What problem was the code intended to solve, and how did the programmer intend for their code to be used?

private final Class sqlClassType;
Copy link
Member

Choose a reason for hiding this comment

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

I had been thinking to add new Type constants in OracleR2dbcTypes:
STRUCT, ARRAY, and REF.

And these SQL types can map to Java classes of the java.sql package: Struct, Array, and Ref

All this is to say that UserDefinedType can retain an io.r2dbc.spi.Type field, rather than a Class field. The Type field would be one of STRUCT, ARRAY, REF. We can then implement getJavaType to return the java type mapping for this Type field.

Does this make sense?

private final String typeName;

public UserDefinedType(Class sqlClassType, String typeName) {
this.sqlClassType = sqlClassType;
this.typeName = typeName;
}

@Override
public Class<?> getJavaType() {
return sqlClassType;
}

@Override
public String getName() {
return typeName;
}
}