oracle / oracle-r2dbc

R2DBC Driver for Oracle Database
https://oracle.com
Other
194 stars 40 forks source link

Support for sending type name with Out Parameters. #83

Closed nirmalhk7 closed 1 year ago

nirmalhk7 commented 2 years ago

As asked here: https://stackoverflow.com/questions/73061887/how-to-specify-parameter-type-in-r2dbc-when-using-stored-procedures

Parameters.out seems to call JDBCs registerOutputParameters within it, but it seems to pass only the binding index and the type (I want to pass the type name too).

Michael-A-McMahon commented 2 years ago

This is a great thing to point out! There is currently no way to bind user defined types (UDT)s.

I think we should first check with the R2DBC SPI project: Should the SPI be enhanced to support UDTs? I'll post an issue on there soon, and we can see where it goes.

It might be the case that UDT support is only valuable for Oracle. In this case, rather than enhance the SPI, Oracle R2DBC should can just extend it. Here's a very rough draft of how we might do that:

class OracleR2dbcTypes {
  ...
  public static final Type STRUCT = ...;

  /**
   * Returns a {@code Type} representing a user defined type.
   * @param type The base type
   * @param name The type name
  public Type userDefined(Type type, String name) {
    ...
  }

This extension would allow us to write code like this:

connection.createStatement("{? = call CREATE_A_PERSON (?)}")
  .bind(0, Parameters.out(
    OracleR2dbcTypes.userDefined(OracleR2dbcTypes.STRUCT, "UDO_PERSON")))
  .bind(1, "YOURE NAME")
  .execute();

@nirmalhk7 and anyone who's interested: Does this seem like a good solution?

nirmalhk7 commented 2 years ago
OracleR2dbcTypes.userDefined(OracleR2dbcTypes.STRUCT, "UDO_PERSON"))

If we do it this way, wont we have to make changes in the Parameters object in R2DBC, so that its able to appropriately process the UDT?

I was thinking more along these lines:

new OracleWrapper(Parameters.out(DATATYPE), "datatype_name"))

We can set appropriate getters to get the parameter, and we wont need to change r2dbc-spi code either. Then in OracleStatementImpl.java#L1381 we can do an if-else check: if the object passed is Parameter type, then we call registerOutParameters the same way, else we call the function and pass an extra parameter (the datatype name) with it (We can get this from the getters in OracleWrapper).

Michael-A-McMahon commented 2 years ago

Thinking about this a bit more. I'm not seeing the need to change the Parameters SPI. I'll flesh out the solution more, please let me know what I'm missing.

The userDefined factory returns an instance of a particular class:

public class UserDefinedType implements Type {
  private final Type baseType;
  private final String name;

  private UserDefinedType(Type baseType, String name) {
    this.baseType = baseType;
    this.name = name;
  }

  public Type baseType() {
    return baseType;
  }

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

  @Override
  public Class<?> getJavaType() {
    // TODO: Map baseType to a Java type
  }
}

An instance of UserDefinedType can be passed to Parameters.out(Type), because UserDefinedType is a subclass of the Type interface.

When Oracle R2DBC goes to bind the parameter:

if (parameter.getType() instanceof UserDefinedType) {
  UserDefinedType userDefinedType = (UserDefineType)parameter.getType();
  SQLType sqlType = toJdbctype(userDefinedType.baseType());
  statement.registerOutParameter(index, sqlType, userDefinedType.getName());
}

I think this works, but maybe your way is better? If I understand it, OracleWrapper is like this:

public class OracleWrapper {
   private final Parameter.Out parameter;
   private final String name;

  ... constructor + getters ...
}

And then the binder code, like what I have above, would be checking instanceof OracleWrapper rather than instanceof UserDefinedType. I guess it's not too different in the end.

I still think I like UserDefinedType better. As a subclass of Type, an instance of this class could be returned by ReadableMetaData.getType(). That seems like something we need to do if we want to support UDTs.

nirmalhk7 commented 1 year ago

Hi, I'd like to take this up. I think I have a solution ready that fits your description @Michael-A-McMahon. I'll create an MR shortly.

Michael-A-McMahon commented 1 year ago

That's awesome @nirmalhk7 ! Please be sure to review our contributors guide: https://github.com/oracle/oracle-r2dbc/blob/main/CONTRIBUTING.md#contributing-code

Thank you!!!