fix: keep DECIMAL parameter scale on the Thrift backend

CHANGELOG.md

@@ -11,6 +11,7 @@
 - Upgrade `thrift` to 0.25, which depends on the CommonJS-compatible `uuid@11` instead of the ESM-only `uuid@13`, so `require('@databricks/sql')` works again in Jest and other environments without `require(esm)`. (#461)
   - **Behavior change for direct users of the exported `thrift.TCLIService`** (`DBSQLClient` itself uses the callback form and is unaffected): `TCLIService.Client` calls made without a callback now return native Promises instead of Q promises, and `TCLIService.Processor` runs promise-returning handlers through native Promises. Q-only methods such as `.fail()`, `.fin()`, `.done()` and `.spread()` no longer exist; use `.catch()`, `.finally()` and `.then()`. An unhandled rejection is now a real Node unhandled rejection, which crashes the process under Node's default `--unhandled-rejections=throw`. Q only emitted the `unhandledRejection` event manually, so it never crashed. Handle every promise a callback-less client call returns.
 - `oauthScopes` is now honored on the Thrift backend: OAuth client-secret M2M sends the configured scopes instead of always `all-apis`, so scoped service-principal secrets can authenticate, and U2M uses them in place of `sql offline_access`. On both backends, entries are split on whitespace and blank values fall back to those defaults. A token read from `persistence` that records different scopes from the ones the client would request is no longer reused or refreshed; the client acquires a new one (for U2M, through a new browser login). Tokens that don't record scopes are still reused. (#540)
+- Fix `DECIMAL` parameters losing their fractional digits on the Thrift backend: `"123.45"` was bound as `decimal(3,0)` and came back as `123`. Both backends now send `DECIMAL(p,s)` derived from the value (also for the `DEC` / `NUMERIC` synonyms), stringify `Int64` values beyond 2^53 exactly instead of as `Infinity`, accept exponent forms such as `1e-7` (rewritten as a plain numeral), and throw a `ParameterError` for non-numeric values or values that need more than 38 digits. (#474)
 
 ## 3.0.0
 

lib/DBSQLParameter.ts

@@ -1,5 +1,6 @@
 import Int64 from 'node-int64';
 import { TSparkParameter, TSparkParameterValue } from '../thrift/TCLIService_types';
+import ParameterError from './errors/ParameterError';
 
 export type DBSQLParameterValue = undefined | null | boolean | number | bigint | Int64 | Date | string;
 
@@ -60,6 +61,77 @@ function inferNumberType(value: number): DBSQLParameterType {
   return DBSQLParameterType.DOUBLE;
 }
 
+const DECIMAL_MAX_PRECISION = 38;
+
+/**
+ * Qualify a bare `DECIMAL` (or its `DEC` / `NUMERIC` synonyms) with the
+ * precision and scale of its value. Both backends otherwise lose the fraction:
+ * the Thrift server binds a scale-0 decimal, truncating `"123.45"` to `123`,
+ * and the kernel's raw-param path forwards the type verbatim, so SEA binds it
+ * as `DECIMAL(10,0)` and rounds.
+ *
+ * Precision is significant integer digits plus scale (Spark's decimal-literal
+ * rule), so `"99.99"` ⇒ `DECIMAL(4,2)` and `"0.00001"` ⇒ `DECIMAL(5,5)`. Plain
+ * numerals are sent unchanged; exponent forms (`1e-7`, which `Number#toString`
+ * produces) are rewritten as the equivalent plain numeral. A non-numeric value
+ * or one needing precision > 38 throws instead of being truncated server-side.
+ */
+function qualifyDecimal(stringValue: string): { type: string; value: string } {
+  const m = /^([+-]?)(\d*)(?:\.(\d*))?(?:[eE]([+-]?\d+))?$/.exec(stringValue.trim());
+  const [, sign = '', intPart = '', fracPart, exponent] = m ?? [];
+  if (!m || (intPart === '' && !fracPart)) {
+    throw new ParameterError(
+      `DECIMAL parameter value "${stringValue}" is not a decimal number (expected e.g. "123.45" or "1.5e-7").`,
+    );
+  }
+
+  const digits = intPart + (fracPart ?? '');
+  const exp = exponent === undefined ? 0 : Number(exponent);
+  // Position of the decimal point within `digits` once the exponent is applied.
+  const pointPos = intPart.length + exp;
+  const leadingZeros = digits.length - digits.replace(/^0+/, '').length;
+  const isZero = leadingZeros === digits.length;
+  const scale = Math.max(digits.length - pointPos, 0);
+  const significantIntDigits = isZero ? 0 : Math.max(pointPos - leadingZeros, 0);
+  const precision = Math.max(significantIntDigits + scale, 1);
+  if (precision > DECIMAL_MAX_PRECISION) {
+    throw new ParameterError(
+      `DECIMAL parameter value "${stringValue}" needs precision ${precision}, exceeding the Databricks maximum of ` +
+        `${DECIMAL_MAX_PRECISION}. Round/scale the value or bind it as a STRING.`,
+    );
+  }
+
+  const type = `DECIMAL(${precision},${scale})`;
+  if (exponent === undefined && fracPart !== '') {
+    return { type, value: stringValue };
+  }
+  // Rebuild as a plain numeral; both padding lengths are bounded by `precision`.
+  let plain: string;
+  if (isZero) {
+    plain = scale > 0 ? `0.${'0'.repeat(scale)}` : '0';
+  } else if (pointPos >= digits.length) {
+    plain = digits + '0'.repeat(pointPos - digits.length);
+  } else if (pointPos <= 0) {
+    plain = `0.${'0'.repeat(-pointPos)}${digits}`;
+  } else {
+    plain = `${digits.slice(0, pointPos)}.${digits.slice(pointPos)}`;
+  }
+  plain = plain.replace(/^0+(?=\d)/, '');
+  return { type, value: sign === '-' ? `-${plain}` : plain };
+}
+
+// Unqualified decimal type names; a qualified `DECIMAL(p,s)` is left as given.
+const BARE_DECIMAL_TYPE = /^(DECIMAL|DEC|NUMERIC)$/i;
+
+/**
+ * Exact decimal string for an `Int64`. `Int64#toString` goes through a JS
+ * number and returns `"Infinity"` for magnitudes ≥ 2^53, so read the
+ * big-endian two's-complement bytes as a `bigint` instead.
+ */
+function int64ToString(value: Int64): string {
+  return value.toBuffer().readBigInt64BE(0).toString();
+}
+
 interface DBSQLParameterOptions {
   type?: DBSQLParameterType;
   value: DBSQLParameterValue;
@@ -79,7 +151,19 @@ export class DBSQLParameter {
     this.value = value;
   }
 
-  public toSparkParameter({ name }: ToSparkParameterOptions = {}): TSparkParameter {
+  public toSparkParameter(options: ToSparkParameterOptions = {}): TSparkParameter {
+    const param = this.toUnqualifiedSparkParameter(options);
+    const stringValue = param.value?.stringValue;
+    // Only an explicit type can be DECIMAL; inferred types never are.
+    if (this.type !== undefined && BARE_DECIMAL_TYPE.test(this.type.trim()) && typeof stringValue === 'string') {
+      const decimal = qualifyDecimal(stringValue);
+      param.type = decimal.type;
+      param.value = new TSparkParameterValue({ stringValue: decimal.value });
+    }
+    return param;
+  }
+
+  private toUnqualifiedSparkParameter({ name }: ToSparkParameterOptions): TSparkParameter {
     // If VOID type was set explicitly - ignore value
     if (this.type === DBSQLParameterType.VOID) {
       return new TSparkParameter({ name }); // for NULL neither `type` nor `value` should be set
@@ -121,7 +205,7 @@ export class DBSQLParameter {
         name,
         type: wireType ?? DBSQLParameterType.BIGINT,
         value: new TSparkParameterValue({
-          stringValue: this.value.toString(),
+          stringValue: this.value instanceof Int64 ? int64ToString(this.value) : this.value.toString(),
         }),
       });
     }

lib/kernel/KernelPositionalParams.ts

@@ -13,57 +13,15 @@
 // limitations under the License.
 
 import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter';
-import ParameterError from '../errors/ParameterError';
 import { KernelNativeRawParameterInput } from './KernelNativeLoader';
 import assertBindableValue from '../utils/assertBindableValue';
 
-/**
- * Derive `(precision,scale)` from a decimal value string for the kernel
- * `DECIMAL(p,s)` type name — the kernel param codec requires the parenthesised
- * form (plain `"DECIMAL"` is rejected) so it preserves the caller's fractional
- * digits. `"99.99"` ⇒ `"4,2"`; `"-123"` ⇒ `"3,0"`; `"0.00001"` ⇒ `"5,5"`.
- *
- * Precision is significant integer digits (insignificant leading zeros
- * stripped, matching Spark's decimal-literal rule) plus the fractional scale.
- * The value is NOT clamped: a non-numeric value, or one that needs precision >
- * the Databricks maximum of 38, throws `ParameterError` at bind time rather
- * than emitting an inconsistent `DECIMAL(38,…)` type alongside an
- * unrepresentable value (which the kernel would reject or silently truncate).
- */
-function decimalPrecisionScale(v: string): string {
-  // Accept an optional sign + plain decimal numeral only. Exponential form
-  // ("1e+21"), empty, and non-numeric ("abc") are rejected with a clear error
-  // instead of mis-deriving (p,s) and surfacing an opaque kernel failure.
-  const m = /^[+-]?(\d*)(?:\.(\d+))?$/.exec(v.trim());
-  const intPart = m?.[1] ?? '';
-  const fracPart = m?.[2] ?? '';
-  if (!m || (intPart === '' && fracPart === '')) {
-    throw new ParameterError(
-      `DECIMAL parameter value "${v}" is not a plain decimal numeral (expected [+-]?digits[.digits]).`,
-    );
-  }
-  const scale = fracPart.length;
-  // Significant integer digits: drop insignificant leading zeros ("007" → 1,
-  // "0" / "" → 0) so e.g. 0.00001 is DECIMAL(5,5), not DECIMAL(6,5).
-  const significantIntDigits = intPart.replace(/^0+/, '').length;
-  const precision = Math.max(significantIntDigits + scale, 1);
-  if (precision > 38) {
-    throw new ParameterError(
-      `DECIMAL parameter value "${v}" needs precision ${precision}, exceeding the Databricks maximum of 38. ` +
-        'Round/scale the value or bind it as a STRING.',
-    );
-  }
-  return `${precision},${scale}`;
-}
-
 /**
  * Reduce a `DBSQLParameter | DBSQLParameterValue` to the napi
  * `RawParameterInput` (`{ name?, sqlType, value? }`) accepted by the kernel's
  * raw-parameter path. Reuses `DBSQLParameter.toSparkParameter` — the same
- * type-inference + value-stringification the Thrift backend uses — then adapts
- * the type name where required:
- * - DECIMAL → `DECIMAL(p,s)` (parenthesised form required)
- * - a missing value ⇒ SQL NULL.
+ * type-inference + value-stringification (including DECIMAL → `DECIMAL(p,s)`)
+ * the Thrift backend uses; a missing value ⇒ SQL NULL.
  */
 function toRawParameterInput(value: DBSQLParameter | DBSQLParameterValue): KernelNativeRawParameterInput {
   const param = value instanceof DBSQLParameter ? value : new DBSQLParameter({ value });
@@ -76,12 +34,7 @@ function toRawParameterInput(value: DBSQLParameter | DBSQLParameterValue): Kerne
     return { sqlType: 'VOID' };
   }
 
-  let sqlType = spark.type ?? 'STRING';
-  const upper = sqlType.toUpperCase();
-  if (upper === 'DECIMAL') {
-    sqlType = `DECIMAL(${decimalPrecisionScale(stringValue)})`;
-  }
-  return { sqlType, value: stringValue };
+  return { sqlType: spark.type ?? 'STRING', value: stringValue };
 }
 
 /**

tests/e2e/kernel/execution-e2e.test.ts

@@ -147,6 +147,34 @@ describe('kernel execution end-to-end', function e2eSuite() {
     }
   });
 
+  it('binds DECIMAL parameters with value-derived precision and scale', async () => {
+    const client = new DBSQLClient();
+
+    await client.connect({
+      host: hostName as string,
+      path: httpPath as string,
+      token: token as string,
+      useKernel: true,
+    } as ConnectionOptions & InternalConnectionOptions);
+
+    const session = await client.openSession({ initialCatalog: 'main' });
+    let operation;
+    try {
+      const decimal = (value: string | number) => new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value });
+      operation = await session.executeStatement('SELECT ? AS a, typeof(?) AS a_type, ? AS b, typeof(?) AS b_type', {
+        // `1e-7` exercises the exponent form `Number#toString` produces.
+        ordinalParameters: [decimal('123.45'), decimal('123.45'), decimal(1e-7), decimal(1e-7)],
+      });
+      expect(await operation.fetchAll()).to.deep.equal([
+        { a: 123.45, a_type: 'decimal(5,2)', b: 1e-7, b_type: 'decimal(7,7)' },
+      ]);
+    } finally {
+      await operation?.close();
+      await session.close();
+      await client.close();
+    }
+  });
+
   it('binds named NULL and empty string through rawParams on the async path', async () => {
     const client = new DBSQLClient();
 

tests/e2e/query_parameters.test.ts

@@ -232,6 +232,48 @@ describe('Query parameters', () => {
     }
   });
 
+  // https://github.com/databricks/databricks-sql-nodejs/issues/474
+  it('should keep the precision and scale of DECIMAL parameters', async () => {
+    const session = await openSession();
+    try {
+      const operation = await session.executeStatement(
+        'SELECT :a AS a, typeof(:a) AS a_type, :b AS b, typeof(:b) AS b_type, :c AS c, typeof(:c) AS c_type',
+        {
+          namedParameters: {
+            a: new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '123.45' }),
+            // `Number#toString` renders this as "-1.5e-7".
+            b: new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: -1.5e-7 }),
+            // More digits than the scale-0 DECIMAL the server used to apply could hold.
+            c: new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: BigInt('12345678901234') }),
+          },
+        },
+      );
+      const result = await operation.fetchAll();
+      await operation.close();
+      expect(result).to.deep.equal([
+        {
+          a: 123.45,
+          a_type: 'decimal(5,2)',
+          b: -1.5e-7,
+          b_type: 'decimal(8,8)',
+          c: 12345678901234,
+          c_type: 'decimal(14,0)',
+        },
+      ]);
+
+      const ordinal = await session.executeStatement('SELECT ? AS v, typeof(?) AS t', {
+        ordinalParameters: [
+          new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '123.45' }),
+          new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '123.45' }),
+        ],
+      });
+      expect(await ordinal.fetchAll()).to.deep.equal([{ v: 123.45, t: 'decimal(5,2)' }]);
+      await ordinal.close();
+    } finally {
+      await session.close();
+    }
+  });
+
   // https://github.com/databricks/databricks-sql-nodejs/issues/261
   describe('non-scalar parameter values (issue #261)', () => {
     // Non-scalar values are rejected client-side (before any RPC is sent), so executeStatement