UDP Implementation #86

nirmalhk7 · 2022-08-22T17:28:36Z

Fixes #83. As explained and discussed in the issue, neither Oracle's R2DBC driver nor R2DBC itself support user defined datatypes within itself. I'm not aware about other databases, but my project used several UDTs within it, and the lack of support is causing me some issues to migrate my project to a reactive stack. Hence, my PR for the same.

I've created a custom class UserDefinedType (Implementing Type class) that records the SQL type to map to (the Java class corresponding it) and the type name (string).

.bind(1, Parameters.Out(new UserDefinedType(java.sql.Array.class, "typeName"))

After this, its relatively straightforward: every time we operate on the Type object, we check if its an instance of UserDefinedType or not and tweak it accordingly.

Signed-off-by: Nirmal Khedkar <[email protected]>

Michael-A-McMahon

Really appreciate you taking this on. Please review my comments and let me know your thoughts/questions.

Michael-A-McMahon · 2022-08-22T20:42:42Z

src/main/java/module-info.java

@@ -36,4 +36,5 @@
  requires transitive r2dbc.spi;

  exports oracle.r2dbc;
+  exports oracle.r2dbc.impl;


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

Michael-A-McMahon · 2022-08-22T20:50:13Z

src/main/java/oracle/r2dbc/impl/OracleStatementImpl.java

@@ -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) {


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.

Michael-A-McMahon · 2022-08-22T20:53:05Z

src/main/java/oracle/r2dbc/impl/OracleReadableImpl.java

@@ -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();


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

Michael-A-McMahon · 2022-08-22T21:06:34Z

src/main/java/oracle/r2dbc/impl/UserDefinedType.java

@@ -0,0 +1,23 @@
+package oracle.r2dbc.impl;


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.

Michael-A-McMahon · 2022-08-22T21:09:03Z

src/main/java/oracle/r2dbc/impl/UserDefinedType.java

+
+import io.r2dbc.spi.Type;
+
+public final class UserDefinedType implements Type {


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?

Michael-A-McMahon · 2022-08-22T21:15:48Z

src/main/java/oracle/r2dbc/impl/UserDefinedType.java

+import io.r2dbc.spi.Type;
+
+public final class UserDefinedType implements Type {
+    private final Class sqlClassType;


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?

Michael-A-McMahon · 2022-10-05T00:53:20Z

Hi @nirmalhk7. Were you planning to return to this? If not, I think I'll have a go at it on my own.

nirmalhk7 · 2022-10-14T07:02:08Z

Hey Michael,
Apologies, I was trying to craft a better way to take this issue.

You can take up this issue, but can you give an estimate when can this be released at the earliest? I think UDPs are a big part of Oracle SQL, so this does warrant a fast release

Michael-A-McMahon · 2022-10-14T16:26:33Z

Thanks for the update. I think I can have a run at this today and, if all goes well, I can get a PR ready for merging.

We are closing in on a 1.1.0 release for Oracle R2DBC. If can get the PR done today, then we can include this in the next release.

I don't want to guess when the release will happen, as I'd probably end up being wrong. However, I can say for sure that we've begun a review and approval process that is required before releasing a new version. It's too hard to say when this will be done at the moment, as most of us at Oracle are busy preparing for the Cloud World and JavaOne conferences happening next week.

Michael-A-McMahon · 2022-11-10T19:31:43Z

Changes in this branch are superseded by #104 .
@nirmalhk7: I very much appreciate the effort you put in here, and your initial issue which got me thinking about this problem. If you have any feedback for the changes in #104, I'd be very interested to hear it.

Final implementation

4737391

Signed-off-by: Nirmal Khedkar <[email protected]>

Michael-A-McMahon requested changes Aug 22, 2022

View reviewed changes

This was linked to issues Nov 10, 2022

option(OracleR2dbcOptions.TLS_SERVER_DN_MATCH, "false") does not work #102

Closed

R2DBC treating warning as exception #93

Closed

This was unlinked from issues Nov 10, 2022

R2DBC treating warning as exception #93

Closed

option(OracleR2dbcOptions.TLS_SERVER_DN_MATCH, "false") does not work #102

Closed

Michael-A-McMahon closed this Jul 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

UDP Implementation #86

UDP Implementation #86

Uh oh!

nirmalhk7 commented Aug 22, 2022

Uh oh!

Michael-A-McMahon left a comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Uh oh!

Michael-A-McMahon Aug 22, 2022

Uh oh!

Michael-A-McMahon Aug 22, 2022

Uh oh!

Michael-A-McMahon Aug 22, 2022

Uh oh!

Michael-A-McMahon Aug 22, 2022

Uh oh!

Michael-A-McMahon Aug 22, 2022

Uh oh!

Michael-A-McMahon commented Oct 5, 2022

Uh oh!

nirmalhk7 commented Oct 14, 2022 •

edited

Loading

Uh oh!

Michael-A-McMahon commented Oct 14, 2022

Uh oh!

Michael-A-McMahon commented Nov 10, 2022

Uh oh!

Uh oh!


		import io.r2dbc.spi.Type;

		public final class UserDefinedType implements Type {

UDP Implementation #86

UDP Implementation #86

Uh oh!

Conversation

nirmalhk7 commented Aug 22, 2022

Uh oh!

Michael-A-McMahon left a comment

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

Michael-A-McMahon commented Oct 5, 2022

Uh oh!

nirmalhk7 commented Oct 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael-A-McMahon commented Oct 14, 2022

Uh oh!

Michael-A-McMahon commented Nov 10, 2022

Uh oh!

Uh oh!

nirmalhk7 commented Oct 14, 2022 •

edited

Loading