[SEA-NodeJS] Fix DECIMAL (p,s) derivation — reject overflow/non-numeric, drop leading-zero miscount

Code-review #412 (gopalldb P1). `decimalPrecisionScale` derived precision by
counting digit characters, which (a) counted insignificant leading zeros
("0.00001" → DECIMAL(6,5)) and (b) clamped precision to 38 while still sending
the full value — so a 40-digit value produced DECIMAL(38,0) alongside an
unrepresentable 40-digit value, which the kernel rejects or silently truncates.
The prior test blessed that clamp as correct (wrong contract).

Now:
- Precision = significant integer digits (insignificant leading zeros stripped,
  matching Spark's decimal-literal rule) + fractional scale. "0.00001" →
  DECIMAL(5,5); "007.50" → DECIMAL(3,2); "0" → DECIMAL(1,0).
- A value needing precision > 38 throws ParameterError at bind time instead of
  clamp-and-send (P1.1).
- A non-numeric / exponential value ("1e+21", "abc", "") throws ParameterError
  with a clear message rather than mis-deriving (p,s) and surfacing an opaque
  kernel error (P1.2 / P2).

Validated live: 0.00001 binds as DECIMAL(5,5) and round-trips; a 40-digit value
is rejected client-side with ParameterError (no server round-trip). Unit tests
updated to assert the new reject contract + leading-zero handling.

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>

Author: msrathore-db

lib/sea/SeaPositionalParams.ts

@@ -13,22 +13,47 @@
 // limitations under the License.
 
 import { DBSQLParameter, DBSQLParameterValue } from '../DBSQLParameter';
+import ParameterError from '../errors/ParameterError';
 import { SeaNativeTypedValueInput, SeaNativeNamedTypedValueInput } from './SeaNativeLoader';
 import assertBindableValue from './SeaInputValidation';
 
 /**
  * Derive `(precision,scale)` from a decimal value string for the SEA
- * `DECIMAL(p,s)` type name — the kernel param codec requires the
- * parenthesised form (plain `"DECIMAL"` is rejected) so it can preserve
- * the caller's fractional digits. `"99.99"` ⇒ `"4,2"`; `"-123"` ⇒ `"3,0"`.
- * Clamped to the Databricks max precision of 38.
+ * `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 {
-  const digits = (v.match(/\d/g) ?? []).length;
-  const dot = v.indexOf('.');
-  const scale = dot < 0 ? 0 : (v.slice(dot + 1).match(/\d/g) ?? []).length;
-  const precision = Math.min(Math.max(digits, 1), 38);
-  return `${precision},${Math.min(scale, precision)}`;
+  // 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}`;
 }
 
 /**

tests/unit/sea/positionalParams.test.ts

@@ -15,6 +15,7 @@
 import { expect } from 'chai';
 import { buildSeaPositionalParams, buildSeaNamedParams } from '../../../lib/sea/SeaPositionalParams';
 import { DBSQLParameter, DBSQLParameterType } from '../../../lib/DBSQLParameter';
+import ParameterError from '../../../lib/errors/ParameterError';
 
 describe('SeaPositionalParams.buildSeaPositionalParams', () => {
   it('returns undefined for no params (keeps the no-options fast path)', () => {
@@ -30,22 +31,36 @@ describe('SeaPositionalParams.buildSeaPositionalParams', () => {
     ]);
   });
 
+  const decimal = (value: string) => () =>
+    buildSeaPositionalParams([new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value })]);
+
   it('emits DECIMAL in the parenthesised DECIMAL(p,s) form the kernel codec requires', () => {
-    expect(
-      buildSeaPositionalParams([new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '99.99' })]),
-    ).to.deep.equal([{ sqlType: 'DECIMAL(4,2)', value: '99.99' }]);
-    expect(
-      buildSeaPositionalParams([new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '-123' })]),
-    ).to.deep.equal([{ sqlType: 'DECIMAL(3,0)', value: '-123' }]);
+    expect(decimal('99.99')()).to.deep.equal([{ sqlType: 'DECIMAL(4,2)', value: '99.99' }]);
+    expect(decimal('-123')()).to.deep.equal([{ sqlType: 'DECIMAL(3,0)', value: '-123' }]);
   });
 
-  it('clamps DECIMAL precision to the Databricks max of 38', () => {
-    // 40 integer digits → precision clamped to 38 (scale 0).
-    expect(
-      buildSeaPositionalParams([
-        new DBSQLParameter({ type: DBSQLParameterType.DECIMAL, value: '1234567890123456789012345678901234567890' }),
-      ]),
-    ).to.deep.equal([{ sqlType: 'DECIMAL(38,0)', value: '1234567890123456789012345678901234567890' }]);
+  it('excludes insignificant leading zeros from precision (Spark decimal-literal rule)', () => {
+    // 0.00001 → DECIMAL(5,5), not DECIMAL(6,5) (the integer "0" is not significant).
+    expect(decimal('0.00001')()).to.deep.equal([{ sqlType: 'DECIMAL(5,5)', value: '0.00001' }]);
+    // "007.50" → significant int "7" (1) + scale 2 ⇒ DECIMAL(3,2).
+    expect(decimal('007.50')()).to.deep.equal([{ sqlType: 'DECIMAL(3,2)', value: '007.50' }]);
+    // "0" ⇒ DECIMAL(1,0) (minimum precision 1).
+    expect(decimal('0')()).to.deep.equal([{ sqlType: 'DECIMAL(1,0)', value: '0' }]);
+  });
+
+  it('rejects a DECIMAL that needs precision > 38 instead of clamping-and-sending', () => {
+    // 40 integer digits can't fit DECIMAL(38,…); clamping the type while sending
+    // the full value is internally inconsistent — reject at bind time instead.
+    expect(decimal('1234567890123456789012345678901234567890')).to.throw(
+      ParameterError,
+      /exceeding the Databricks maximum of 38/,
+    );
+  });
+
+  it('rejects a non-numeric / exponential DECIMAL value', () => {
+    expect(decimal('1e+21')).to.throw(ParameterError, /not a plain decimal numeral/);
+    expect(decimal('abc')).to.throw(ParameterError, /not a plain decimal numeral/);
+    expect(decimal('')).to.throw(ParameterError, /not a plain decimal numeral/);
   });
 
   it('collapses every INTERVAL subtype to the kernel codec\'s single "INTERVAL" type name', () => {