Added support for FIXED types #261

he-is-harry · 2025-03-12T14:15:58Z

Added functions to read and write FIXED8, FIXED12, and FIXED16 types
Modified Statement.js and Writer.js to pass the fraction metadata to the Writer for use in writing FIXED types
Added unit tests for the new FIXED type functions in bignum.js, Reader.js and Writer.js
Added integration tests for the new FIXED types

- Added support for BINTEXT and BOOLEAN for data format version 7 - BINTEXT is sent and received as buffers - BOOLEAN can be sent as strings, numbers, and boolean and received as boolean - Modified data type integration tests to skip tests when the data format version is lower than required - Added integration tests for BINTEXT and BOOLEAN - Added unit tests for BOOLEAN

- Added spatialTypes option to allow strings to be sent for spatial types - With spatialTypes=0 (default), strings inputted for ST_GEOMETRY and ST_POINT are interpreted as hex - With spatialTypes=1, strings inputted for ST_GEOMETRY and ST_POINT can be encoded in utf8 or cesu8 - Buffers can be sent in any of these options and will be interpreted as binary - Added integration tests to check inserting and fetching large ST_GEOMETRY data - Added some integration tests to check the spatialTypes=1 behaviour

- Changed the Writer to return an error message of 'Argument must be a string or Buffer' when an invalid type is entered for spatial types - Added some hex string and buffer inputs to ST_POINT tests - Changed some inputs for integration tests to describe what is tested more accurately

- Merge changes for data format version 5 into this branch to resolve merge conflicts

- Added support for BINTEXT to be entered as a string - As a consequence, strings can be inputted for all BLOB types (consistent with hana-client) - Added normalized typecode for BINTEXT - Updated data type integration tests to automatically add a ID column to all tables. Now, expected outputs will be in the same order as input

- Added functions to read and write FIXED8, FIXED12, and FIXED16 types - Modified Statement.js and Writer.js to pass the fraction metadata to the Writer for use in writing FIXED types - Added unit tests for the new FIXED type functions in bignum.js, Reader.js and Writer.js - Added integration tests for the new FIXED types

- Added a description for data format version 8 and its changes - Increased timeout on error tests for FIXED types to reduce sporadic timeout failures

- Merged changes from SAP#259 and SAP#254 into fixed8-12-16

IanMcCurdy · 2025-03-12T20:21:38Z

lib/protocol/Writer.js

@@ -503,6 +508,100 @@ Writer.prototype[TypeCode.DECIMAL] = function writeDecimal(value) {
  this.push(buffer);
 };

+function fixedToDecimal(value, fraction, typeStr) {


I think toFixedDecimal would be a better name since we aren't converting a 'fixed' value to a decimal, but rather converting a general value to a decimal with a particular format.

IanMcCurdy · 2025-03-12T20:45:37Z

lib/protocol/Writer.js

@@ -39,12 +40,14 @@ var REGEX = {
 };

 const maxDecimalMantissaLen = 34;
+const maxFixedMantissaLen = 38;



Maybe add a comment here describing the structure of 'params'

IanMcCurdy · 2025-03-14T18:55:09Z

lib/protocol/Writer.js

@@ -870,12 +969,12 @@ function trimTrailingZeroes(str) {
  return str.substring(0, i + 1);
 }

-function stringToDecimal(str) {
+function stringToDecimal(str, maxMantissaLen, minExp, typeStr) {


In my own testing I discovered a subtle behaviour change. Previously, inserting to a DECIMAL column would truncate decimal values beyond the column scale, whereas now with DFV 8 support these values are being rounded. For example, inserting "123.456" into a DECIMAL(10,2) gives "123.45" with DFV 7, but "123.46" with DFV 8. Both the hana-client driver and the server itself use truncation, and I found a reference to this in the server documentation, so we should treat that as the correct behaviour.

IanMcCurdy · 2025-03-14T20:22:01Z

test/fixtures/parametersData.js

@@ -432,6 +432,131 @@ exports.DECIMAL = {
  ]
 };

+exports.FIXED = {


Should add a few exponential form string inputs (e.g. "123e+12").

IanMcCurdy · 2025-03-14T20:29:38Z

test/lib.Writer.js

      Writer.prototype.setValues.bind(writer, [false]).should.throw();
      // Regex does not match
      Writer.prototype.setValues.bind(writer, ['1^6']).should.throw();
      Writer.prototype.setValues.bind(writer, ['']).should.throw();
      Writer.prototype.setValues.bind(writer, ['.']).should.throw();
    });

+    it('should raise wrong input type error for FIXED8', function () {


You might be able to hit currently untested code paths in two possible ways using exponential form string inputs (i.e. "123e12"):

exponential form where the exponent puts the value over the max precision

exponential form where the exponent puts the value under the max precision, but adding additional 0s to the mantissa exceeds the max precision. (e.g. "99e+12" -> DECIMAL(15, 5) would overflow on the client side because the mantissa becomes 19 digits long).

These apply to the fixed12/16 tests as well

IanMcCurdy · 2025-03-14T20:32:00Z

test/acceptance/db.DataType.js

@@ -822,4 +822,126 @@ describe('db', function () {
      async.each(invalidTestData, testDataTypeError.bind(null, 'BOOLEAN_TABLE'), done);
    });
  });
+
+  describeRemoteDB('FIXED', function () {


I'd like to see these integration tests run with DFV 7 as well so we can make sure there's no behaviour changes (sending the values to the server as DECIMAL vs FIXED types should give the same results).

- Changed the behaviour when writing fixed DECIMAL types with inputs larger than the max mantissa length to truncate rather than rounding - The behaviour for writing floating DECIMAL types larger than the max mantissa length remains to round (consistent with HANA's TO_DECIMAL) - Changed FIXED type writing to truncate rather than round - Added a zero to the zeropad.js to fix a case where large values inserted from FIXED types could yield '1undefined' when returning as a DECIMAL - Added unit and integration tests for exponential forms for FIXED types, and the new fixed decimal truncation behaviour - Modified the integration tests for FIXED types to also run on DFV 7 to ensure the same behaviour for DECIMAL types.

he-is-harry added 8 commits January 22, 2025 10:08

Merge branch 'spatialTypesOption' into binTextSupport

6e7e6bb

- Merge changes for data format version 5 into this branch to resolve merge conflicts

Merge branch 'master' into binTextSupport

5dd8349

Added data format version 8 description

2b5b596

- Added a description for data format version 8 and its changes - Increased timeout on error tests for FIXED types to reduce sporadic timeout failures

he-is-harry force-pushed the fixed8-12-16 branch from f14cc71 to 2b5b596 Compare March 12, 2025 15:06

Merge branch 'master' into fixed8-12-16

0cc6e7e

- Merged changes from SAP#259 and SAP#254 into fixed8-12-16

IanMcCurdy requested changes Mar 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for FIXED types #261

Added support for FIXED types #261

he-is-harry commented Mar 12, 2025

IanMcCurdy Mar 12, 2025

IanMcCurdy Mar 12, 2025

IanMcCurdy Mar 14, 2025

IanMcCurdy Mar 14, 2025

IanMcCurdy Mar 14, 2025

IanMcCurdy Mar 14, 2025

Added support for FIXED types #261

Are you sure you want to change the base?

Added support for FIXED types #261

Conversation

he-is-harry commented Mar 12, 2025

IanMcCurdy Mar 12, 2025

Choose a reason for hiding this comment

IanMcCurdy Mar 12, 2025

Choose a reason for hiding this comment

IanMcCurdy Mar 14, 2025

Choose a reason for hiding this comment

IanMcCurdy Mar 14, 2025

Choose a reason for hiding this comment

IanMcCurdy Mar 14, 2025

Choose a reason for hiding this comment

IanMcCurdy Mar 14, 2025

Choose a reason for hiding this comment