php-sap / saprfc-kralik

PHP/SAP implementation for Gregor Kraliks sapnwrfc module
https://php-sap.github.io/
MIT License
10 stars 2 forks source link

RFCTYPE_BCD Data Type Changed from Float to String Causing Inconsistencies in SAP Data #30

Closed ynnoig closed 2 months ago

ynnoig commented 2 months ago

In the latest version (5.x) of the php-sap/saprfc-kralik library, the RFCTYPE_BCD SAP Netweaver RFC type has been mapped to IValue::TYPE_STRING. This is a change from the previous version (4.x), where RFCTYPE_BCD was mapped to a float.

This change introduces significant issues when working with certain SAP data types such as CURR (Currency) and QUAN (Quantity), which represent monetary values and quantities respectively. Instead of being parsed as floats, they are now being returned as strings.

This behavior is inconsistent with the underlying SAP RFC type definition, where RFCTYPE_BCD should correspond to a float value, as seen in the official gkralik/php7-sapnwrfc library:

Affected Code:

The change in the php-sap/saprfc-kralik library is located in the \phpsap\saprfc\Traits\ApiTrait::mapType method, where the RFCTYPE_BCD is mapped to IValue::TYPE_STRING instead of IValue::TYPE_FLOAT:

private function mapType(string $type): string
{
    $mapping = [
        'RFCTYPE_DATE'      => IValue::TYPE_DATE,
        'RFCTYPE_TIME'      => IValue::TYPE_TIME,
        'RFCTYPE_INT'       => IValue::TYPE_INTEGER,
        'RFCTYPE_NUM'       => IValue::TYPE_INTEGER,
        'RFCTYPE_INT1'      => IValue::TYPE_INTEGER,
        'RFCTYPE_INT2'      => IValue::TYPE_INTEGER,
        'RFCTYPE_INT8'      => IValue::TYPE_INTEGER,
        'RFCTYPE_BCD'       => IValue::TYPE_STRING,  // Incorrect mapping
        'RFCTYPE_FLOAT'     => IValue::TYPE_FLOAT,
        'RFCTYPE_CHAR'      => IValue::TYPE_STRING,
        'RFCTYPE_STRING'    => IValue::TYPE_STRING,
        'RFCTYPE_BYTE'      => IValue::TYPE_HEXBIN,
        'RFCTYPE_XSTRING'   => IValue::TYPE_HEXBIN,
        'RFCTYPE_STRUCTURE' => IStruct::TYPE_STRUCT,
        'RFCTYPE_TABLE'     => ITable::TYPE_TABLE
    ];
    if (!array_key_exists($type, $mapping)) {
        throw new SapLogicException(sprintf('Unknown SAP Netweaver RFC type \'%s\'!', $type));
    }
    return $mapping[$type];
}

Problematic Behavior:

In the previous version (4.x), fields like CURR and QUAN were returned as floats, which is the expected behavior. However, after the update in version 5.x, these fields are now returned as strings, leading to inaccurate handling of numeric values in downstream processes.

Here are examples of the data before and after the change:

Impact:

Suggested Fix: The RFCTYPE_BCD should be mapped back to a float (IValue::TYPE_FLOAT) to ensure consistency with the SAP RFC type definition and the correct handling of numeric values.

Reference to the correct behavior:

Please investigate and restore the correct handling of RFCTYPE_BCD to a float.

ynnoig commented 2 months ago

Update: after reading the usage documentation of gkralik/php7-sapnwrfc it seems that this change was also there performed https://github.com/gkralik/php7-sapnwrfc/blob/main/docs/usage.rst https://github.com/gkralik/php7-sapnwrfc/commit/9073bdfb74c3a9a683fd3bfa4a2e6fef3ee4dadf

But i don't know if it is correct that this numeric fields like price (CURR) or quantity (QUAN) are handles/casted to string.

gregor-j commented 2 months ago

explanation

According to SAP documentation BCD is a numerical value of up to 16 digits:

RFCTYPE_BCD = 2, /* packed number, any length between 1 and 16 bytes */

source: https://help.sap.com/doc/saphelp_em92/9.2/en-US/48/a862055135307ce10000000a42189b/frameset.htm

Contrary to that PHPs float is a numerical value of up to 14 digits:

The size of a float is platform-dependent, although a maximum of approximately 1.8e308 with a precision of roughly 14 decimal digits is a common value (the 64 bit IEEE format).

source: https://www.php.net/manual/en/language.types.float.php

Using Kraliks SAPNWRFC module prior to 2.0.0 would cause an error if the SAP BCD value exceeded PHP float. Version 2.0.0 of SAPNWRFC therefore introduced a breaking change:

  • Map RFC type BCD to string BC break

source: https://github.com/gkralik/php7-sapnwrfc/releases/tag/2.0.0

conclusion

Treating BCD values as string, bypasses the issue of exceeding bytes in PHP. Therefore no fix will be issued.