oracle / odpi

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

Option to get json float as float. It is got as double now. #174

Closed kubo closed 1 year ago

kubo commented 2 years ago
  1. Describe your new request in detail

Could you add #define DPI_JSON_OPT_FLOAT_AS_FLOAT 0x04 to dpi.h and change https://github.com/oracle/odpi/blob/515a426239cfbd37d9446def11d0c5f8f1bb808b/src/dpiJson.c#L263-L266 as follows?

        case DPI_JZNVAL_FLOAT:
            node->oracleTypeNum = DPI_ORACLE_TYPE_NUMBER;
            if (options & DPI_JSON_OPT_FLOAT_AS_FLOAT) {
              node->nativeTypeNum = DPI_NATIVE_TYPE_FLOAT;
              node->value->asFloat = scalar.value.asFloat.value;
            } else {
              node->nativeTypeNum = DPI_NATIVE_TYPE_DOUBLE;
              node->value->asDouble = scalar.value.asFloat.value;
            }

When a float value is set to json, I want to get it as float without conversion from float to double, though I suspect that nobody uses float in json.

In rust language, double and float literals 2.3 are printed as 2.3. But the double value converted from float 2.3 is printed as 2.299999952316284. I want to avoid this kind of type conversion by DPI_JSON_OPT_FLOAT_AS_FLOAT.

    let float_val: f32 = 2.3;
    let double_val: f64 = 2.3;
    let double_from_float_val: f64 = float_val.into();

    println!("float_val: {}", float_val);
    // => float_val: 2.3
    println!("double_val: {}", double_val);
    // => double_val: 2.3
    println!("double_from_float_val: {}", double_from_float_val);
    // => double_from_float_val: 2.299999952316284
tgulacsi commented 1 year ago

See https://stackoverflow.com/a/17508660 for the reason.

So convert to float and print that.

kubo commented 1 year ago

So convert to float and print that.

Agree. However there is no way to know whether a double value should be converted to float or not.

Both DPI_JZNVAL_FLOAT and DPI_JZNVAL_DOUBLE are got as same type. The former should be converted to float to print it. The latter should not. However there is no way to distinguish them. https://github.com/oracle/odpi/blob/515a426239cfbd37d9446def11d0c5f8f1bb808b/src/dpiJson.c#L263-L272

anthony-tuininga commented 1 year ago

This could be addressed by having node->nativeTypeNum returned as DPI_NATIVE_TYPE_FLOAT instead. Would that address your concerns?

kubo commented 1 year ago

node->nativeTypeNum = DPI_NATIVE_TYPE_FLOAT for DPI_JZNVAL_FLOAT is a must to address this.

In addition when nativeTypeNum is DPI_NATIVE_TYPE_FLOAT, scalar.value.asFloat.value should be set to node->value->asFloat for consistency of nativeTypeNum and node->value member. It isn't a must though.

Changing nativeTypeNum is incompatible change. So I proposed to add the option to change the behavior.

anthony-tuininga commented 1 year ago

Adding the option makes sense. I'll look into that when I get back from holidays!

anthony-tuininga commented 1 year ago

Thinking about this a bit more, I think I'd rather just change the value of nativeTypeNum to be DPI_NATIVE_TYPE_FLOAT when the value is DPI_JZNVAL_FLOAT as that makes more sense. This does mean that existing drivers that assumed that values would always come back as DPI_NATIVE_TYPE_DOUBLE will need to be changed. As such, I think I'll relabel current development to 5.0 and note the need for adjusting driver assumptions for processing JSON. Use of DPI_JSNVAL_FLOAT is uncommon, too. Thoughts?

anthony-tuininga commented 1 year ago

@kubo, I've pushed the change I suggested and also bumped the version to 5.0. Timing on the release of 5.0 is unknown at this point -- but if you're not concerned about an official release, this should get you going!

kubo commented 1 year ago

@anthony-tuininga Thanks. It works fine.