Skip to content
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

Ojdbc provider jackson oson #118

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

fmeheust
Copy link
Member

No description provided.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 12, 2024
Copy link
Member

@jjspiegel jjspiegel left a comment

Choose a reason for hiding this comment

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

Looks fantastic, thank you for doing this. Most of my comments are trivial, please see the one about UUID serialization though.

public static void createTable(Connection conn) throws SQLException {
try (Statement stmt = conn.createStatement()) {
stmt.addBatch("drop table if exists jackson_oson_sample");
stmt.addBatch("create table jackson_oson_sample(id number, json_value JSON) tablespace tbs1");
Copy link
Member

Choose a reason for hiding this comment

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

I suggest removing "tablespace tbs1" and let it use the default tablespace because most databases may not have this by default. I had to remove this to get the examples to run.

* implementation that works with Oracle's JSON generator {@link OracleJsonGenerator}. This class
* supports writing JSON data with various data types.
*/
public class OsonGenerator extends GeneratorBase {
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a serializer for UUID? Right now it is serializing as a string. Instead it would be better if by default it was converted to a byte array and written using gen.writeId(byte[]).

@Override
public void writeUTF8String(byte[] buffer, int offset, int len) throws IOException {
_verifyValueWrite("writeUTF8String");
gen.write(new String(buffer, offset, len, StandardCharsets.UTF_8));
Copy link
Member

Choose a reason for hiding this comment

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

OsonParserImpl and OsonGeneratorImpl have the ability to do UTF8 passthrough. See:
https://codesearch.oraclecorp.com/cs/xref/RDBMS_MAIN_LINUX.X64/dbjava/src/java/oracle/jdbc/driver/json/binary/OsonGeneratorImpl.java#2175

It would be great if we expose these methods and use them here to avoid string construction.

Copy link
Member Author

Choose a reason for hiding this comment

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

These will have to be done on another version, since it would require changes to the driver

@@ -0,0 +1,57 @@
# OSON Provider for Jackson
Copy link
Member

Choose a reason for hiding this comment

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

Nice README. It would be great if you could include a definition of the type mappings, i.e. a table with one column that lists Java types and the default OSON mapping.

Also if I want to override a mapping for some field and gain access to the underlying OracleJsonParser and OracleJsonGenerator is this possible?

protected void _verifyValueWrite(String typeMsg) throws IOException {
int status = _writeContext.writeValue();
if(status == _writeContext.STATUS_EXPECT_NAME) {
_reportError("error: expecting value. Got name: " + typeMsg);
Copy link
Member

Choose a reason for hiding this comment

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

Should these strings be externalized for translation?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now error messages are not translated in the extensions.

* of the JSON object.
* </p>
*/
public class AccessJsonColumnUsingJacksonObjectNode {
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 that this example should point out that this is only recommended if some existing API requires to consume the data as ObjectNode. Otherwise, it would be better to use OracleJsonObject as this will provide in-place reads of the underlying data without making a copy and it will better expose the underlying type system.

public static void main(String[] args) {
try {
// set the system property to the class implementing the "oracle.jdbc.spi.JsonProvider interface"
System.setProperty(OracleConnection.CONNECTION_PROPERTY_PROVIDER_JSON, JacksonOsonProvider.PROVIDER_NAME);
Copy link
Member

Choose a reason for hiding this comment

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

Does this have to be done by system property?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it can also be added as connection property, I have changed the samples.

System.setProperty(OracleConnection.CONNECTION_PROPERTY_PROVIDER_JSON, JacksonOsonProvider.PROVIDER_NAME);

Connection conn = JacksonOsonSampleUtil.createConnection();
JacksonOsonSampleUtil.createTable(conn);
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to include an example that shows:

  • Nested structures
  • Extended types (like OffsetDateTime)
  • Selecting a value based on a filter within the JSON

Also this feature demos nicely with JSON collection tables and Java records. Here is one simple example I tried:

public record Image(    
    String location,
    String description
) {}
import java.math.BigDecimal;
import java.time.OffsetDateTime;
import java.util.List;

import com.fasterxml.jackson.annotation.JsonProperty;

public record Movie(
    @JsonProperty("_id")
    int id,
    String title,
    String genre,
    BigDecimal gross,
    OffsetDateTime released,
    List<Image> images
) {}
import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.time.OffsetDateTime;
import java.util.List;

import oracle.jdbc.OracleConnection;
import oracle.jdbc.OracleType;

public class MovieTest {

    public static void main(String[] args) throws SQLException {
        System.setProperty(OracleConnection.CONNECTION_PROPERTY_JSON_DEFAULT_GET_OBJECT_TYPE, "oracle.sql.json.OracleJsonValue");
        try (Connection con = DriverManager.getConnection("....")) {
            Statement stmt = con.createStatement();
            stmt.execute("drop table if exists movie");
            stmt.execute("create json collection table movie");
            
            Movie matrix = new Movie(
                    123, 
                    "The Matrix", 
                    "Sci-fi", 
                    BigDecimal.valueOf(353212323), 
                    OffsetDateTime.now(),
                    List.of(
                        new Image("poster.png", "The Matrix"),
                        new Image("showtimes.png", "Matrix Showtimes")
                    ) 
            );
            
            Movie shawshank = new Movie(
                    456, 
                    "The Shawshank Redemption", 
                    "Drama", 
                    BigDecimal.valueOf(453212345), 
                    OffsetDateTime.now(),
                    null
            );
            
            PreparedStatement insert = con.prepareStatement("insert into movie values (:1)");
            insert.setObject(1, matrix, OracleType.JSON);
            insert.execute();
            
            insert.setObject(1, shawshank, OracleType.JSON);
            insert.execute();
            
            ResultSet rs = stmt.executeQuery("select * from movie m where m.data.title = 'The Matrix'");
            rs.next();
            Movie result = rs.getObject(1, Movie.class);
            System.out.println(result);
        }   
    }
}
SQL> select json_serialize(data pretty) from movie;
{
  "_id" : 123,
  "title" : "The Matrix",
  "genre" : "Sci-fi",
  "gross" : 353212323,
  "released" : "2024-11-20T19:35:02.941012-08:00",
  "images" :
  [
    {
      "location" : "poster.png",
      "description" : "The Matrix"
    },
    {
      "location" : "showtimes.png",
      "description" : "Matrix Showtimes"
    }
  ]
}

{
  "_id" : 456,
  "title" : "The Shawshank Redemption",
  "genre" : "Drama",
  "gross" : 453212345,
  "released" : "2024-11-20T19:35:02.941256-08:00",
  "images" : null
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for the code sample, I have added it (with small changes to match JDK version).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants