oracle / odpi

ODPI-C: Oracle Database Programming Interface for Drivers and Applications
https://oracle.github.io/odpi/
Other
268 stars 78 forks source link

OCINumber as a native type #4

Closed kubo closed 7 years ago

kubo commented 7 years ago

Would you have a plan to expose OCINumber as a native type? It is required for drivers which implement JDBC-like API.

Assume the following code:

ResultSet rs = stmt.executeQuery("select NUMBER_COL from TABLE_NAME");
while (rs.next()) {
     ... use rs.getInt(1), rs.getLong(1), rs.getDouble(1) or rs.getBigDecimal(1) ...
}

The native type of NUMBER_COL is not decided at stmt.executeQuery(). It depends on methods such as getInt after the column data is fetched. However such API cannot be implemented by ODPI-C, which must declare native types explicitly or implicitly at SQL execution time.

If ODPI-C exposes OCINumber as a native type and provides conversion functions between OCINumber and integer, double and text, drivers can convert the OCINumber to int, long, double and BigDecimal via text after it is fetched.

anthony-tuininga commented 7 years ago

There is no plan to expose OCINumber as a native type but that can be considered. ODPI-C currently performs the conversion for you automatically from OCINumber to integer, double or string depending on the value of dpiNativeTypeNum that you select when creating the variable. This is either known in advance (and explicitly specified) or can be determined by examining the metadata. In any case, that is what the drivers that I have migrated to use ODPI-C currently do. This reduces the number of calls that need to be made since the conversion happens internally during the fetch. Do you have a particular driver in mind?

kubo commented 7 years ago

This comment was edited.

I'm now writing a Rust driver. Driver users must write the following code when they try to fetch a number column as integer.

// This API isn't fixed. I bet that it will be changed later.
let stmt = conn.prepare("select number_col form some_table");
// The number_col must be defined explicitly. Otherwise, ODPI-C defines it as double implicitly.
// The driver cannot guess what method is called later to get the column data.
stmt.define(1, oracle::NativeType::Integer);
stmt.execute();
let row = stmt.fetch();
row.get::<i64>(1); // Get the first column as 64 bit integer.

If OCINumber is exposed as a native type, the code become a bit simple as follows.

let stmt = conn.execute("select col form some_table"); // No need to define the number column as integer.
let row = stmt.fetch();
row.get::<i64>(1); // The driver converts OCINumber to 64 bit integer internally.

I won't permit row.get::<...>(1) for all Rust data types. But I want to permit it for various Rust number data types as long as the Oracle data type is number.

anthony-tuininga commented 7 years ago

Why can't you examine the metadata in your driver? If the scale is zero and the precision is a positive number less than or equal to DPI_MAX_INT64_PRECISION, the value can be converted to a 64-bit integer. If the scale is zero and the precision is greater than DPI_MAX_INT64_PRECISION it can be converted to a big integer (via string). For all other cases double makes the most sense. I'm not sure what the advantage would be to force the end user to make that decision instead?

My suggestion would be to use the above heuristic as a default and allow the end user to override that only if they need to do so -- and most of the time that should not be necessary. The method row.get() should then simply return the correct type without having to specify row.get::<i64>. That is certainly the approach I have seen in other Rust drivers. It is also the approach I have taken in my Python driver (which is not yet available on GitHub).

kubo commented 7 years ago

End users must decide the type returned by row.get() in any case. Even though they don't specify it explicitly, the rust compiler infers it and converts row.get() to row.get::<type>() implicitly. The body of the get method must check whether the native type matches with the return type and convert it if necessary. Otherwise, compilation stops with a type mismatch error.

Example of type inference:

let col_data = row.get(1);
... pass col_data to an i64 argument of a function ...

The row.get(1) is shorthand of row.get::<i64>(1) and must return i64. The rust compiler infers col_data as i64 by checking how the variable is used later.

kubo commented 7 years ago

I don't request you to change the default native type of number to OCINumber. I agree that double is preferable in general. However it is not preferable for my driver. My driver will examine the metadata just before dpiStmt_execute and define number columns as OCINumber (or string) if they are not defined explicitly. If they are defined as double implicitly and the return type of row.get() is inferred as i64 without intent, the return value may be incorrect because the precision of double is 15 in decimals, which is smaller than the precision of i64.

anthony-tuininga commented 7 years ago

The default native type is changed to integer if it the number can be placed in a 64-bit integer and is otherwise returned as double. You can adjust this to return integer, double or text as needed. This is all that OCI supports and should be sufficient for your needs, too. Feel free to reopen this discussion if desired.