Skip to content

Commit e4ce6ee

Browse files
committed
fix: preserve kernel decimal precision and scale metadata
Signed-off-by: Cathleen Yan <58714163+cathleeny@users.noreply.github.com>
1 parent 13c9a71 commit e4ce6ee

3 files changed

Lines changed: 95 additions & 3 deletions

File tree

‎lib/kernel/KernelArrowIpc.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// limitations under the License.
1414

1515
import { RecordBatchReader, MessageReader, MessageHeader, Schema, Field, DataType, TypeMap } from 'apache-arrow';
16-
import { TTableSchema, TTypeId, TPrimitiveTypeEntry } from '../../thrift/TCLIService_types';
16+
import { TTableSchema, TTypeId, TPrimitiveTypeEntry, TTypeQualifiers } from '../../thrift/TCLIService_types';
1717
import { rewriteDurationToInt64, DURATION_UNIT_METADATA_KEY } from './KernelArrowIpcDurationFix';
1818
import HiveDriverError from '../errors/HiveDriverError';
1919

@@ -264,6 +264,20 @@ export function arrowSchemaToThriftSchema(arrowSchema: Schema<TypeMap>): TTableS
264264
const primitiveEntry: TPrimitiveTypeEntry = {
265265
type: arrowTypeToTTypeId(field),
266266
};
267+
if (primitiveEntry.type === TTypeId.DECIMAL_TYPE) {
268+
const qualifiers: TTypeQualifiers['qualifiers'] = {};
269+
const decimal = DataType.isDecimal(field.type) ? field.type : undefined;
270+
for (const key of ['precision', 'scale'] as const) {
271+
// Prefer manifest qualifiers, including for text-encoded decimals.
272+
// Native Arrow decimals also carry these when metadata is absent.
273+
const text = field.metadata.get(`databricks.type_${key}`);
274+
const parsed = text !== undefined && /^-?\d+$/.test(text) ? Number(text) : NaN;
275+
const minimum = key === 'precision' ? 1 : -0x80000000;
276+
const value = Number.isInteger(parsed) && parsed >= minimum && parsed <= 0x7fffffff ? parsed : decimal?.[key];
277+
if (value !== undefined) qualifiers[key] = { i32Value: value };
278+
}
279+
if (Object.keys(qualifiers).length > 0) primitiveEntry.typeQualifiers = { qualifiers };
280+
}
267281
return {
268282
columnName: field.name,
269283
typeDesc: {

‎tests/unit/kernel/KernelArrowIpc.test.ts‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,14 @@
1313
// limitations under the License.
1414

1515
import { expect } from 'chai';
16-
import { tableFromArrays, RecordBatchStreamWriter } from 'apache-arrow';
17-
import { countRowsInIpc, decodeIpcSchema, patchIpcBytes } from '../../../lib/kernel/KernelArrowIpc';
16+
import { tableFromArrays, RecordBatchStreamWriter, Schema, Field, Decimal, Utf8, Int32 } from 'apache-arrow';
17+
import {
18+
arrowSchemaToThriftSchema,
19+
countRowsInIpc,
20+
decodeIpcSchema,
21+
patchIpcBytes,
22+
} from '../../../lib/kernel/KernelArrowIpc';
23+
import { TTypeId } from '../../../thrift/TCLIService_types';
1824
import { rewriteDurationToInt64 } from '../../../lib/kernel/KernelArrowIpcDurationFix';
1925
import HiveDriverError from '../../../lib/errors/HiveDriverError';
2026

@@ -100,3 +106,57 @@ describe('KernelArrowIpc.decodeIpcSchema', () => {
100106
expect(() => decodeIpcSchema(Buffer.alloc(0))).to.throw(HiveDriverError);
101107
});
102108
});
109+
110+
describe('KernelArrowIpc.arrowSchemaToThriftSchema decimal qualifiers', () => {
111+
for (const [precision, scale] of [
112+
[10, 2],
113+
[38, 0],
114+
[38, 38],
115+
[10, -2],
116+
]) {
117+
it(`preserves Arrow DECIMAL(${precision},${scale}) precision and scale`, () => {
118+
const schema = new Schema([new Field('amount', new Decimal(scale, precision, 128), true)]);
119+
expect(arrowSchemaToThriftSchema(schema).columns[0].typeDesc.types[0].primitiveEntry).to.deep.equal({
120+
type: TTypeId.DECIMAL_TYPE,
121+
typeQualifiers: { qualifiers: { precision: { i32Value: precision }, scale: { i32Value: scale } } },
122+
});
123+
});
124+
}
125+
126+
for (const type of [new Utf8(), new Decimal(2, 10, 128)]) {
127+
it(`prefers logical decimal qualifiers over the ${type} physical type`, () => {
128+
const metadata = new Map([
129+
['databricks.type_name', 'DECIMAL'],
130+
['databricks.type_precision', '20'],
131+
['databricks.type_scale', '0'],
132+
]);
133+
const schema = new Schema([new Field('amount', type, true, metadata)]);
134+
expect(arrowSchemaToThriftSchema(schema).columns[0].typeDesc.types[0].primitiveEntry).to.deep.equal({
135+
type: TTypeId.DECIMAL_TYPE,
136+
typeQualifiers: { qualifiers: { precision: { i32Value: 20 }, scale: { i32Value: 0 } } },
137+
});
138+
});
139+
}
140+
141+
for (const invalid of ['', 'invalid', '10.5', '2147483648']) {
142+
it(`falls back to Arrow for invalid decimal qualifiers ${JSON.stringify(invalid)}`, () => {
143+
const metadata = new Map([
144+
['databricks.type_precision', invalid],
145+
['databricks.type_scale', invalid],
146+
]);
147+
const schema = new Schema([new Field('amount', new Decimal(2, 10, 128), true, metadata)]);
148+
expect(
149+
arrowSchemaToThriftSchema(schema).columns[0].typeDesc.types[0].primitiveEntry?.typeQualifiers,
150+
).to.deep.equal({ qualifiers: { precision: { i32Value: 10 }, scale: { i32Value: 2 } } });
151+
});
152+
}
153+
154+
it('does not invent missing qualifiers or attach decimal qualifiers to other types', () => {
155+
const schema = new Schema([
156+
new Field('unknown_decimal', new Utf8(), true, new Map([['databricks.type_name', 'DECIMAL']])),
157+
new Field('integer', new Int32(), true, new Map([['databricks.type_precision', '10']])),
158+
]);
159+
const entries = arrowSchemaToThriftSchema(schema).columns.map((column) => column.typeDesc.types[0].primitiveEntry);
160+
expect(entries).to.deep.equal([{ type: TTypeId.DECIMAL_TYPE }, { type: TTypeId.INT_TYPE }]);
161+
});
162+
});

‎tests/unit/kernel/KernelOperationBackend.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,11 @@ describe('KernelOperationBackend — M0 datatype round-trip via napi → ArrowRe
211211
expect((row.bin as Buffer).equals(Buffer.from([0xde, 0xad, 0xbe, 0xef]))).to.equal(true);
212212
// DECIMAL: Phase-1 scale-aware coercion via Arrow's Decimal type → 1.5
213213
expect(row.dec).to.equal(1.5);
214+
const metadata = await backend.getResultMetadata();
215+
const decimalColumn = metadata.schema?.columns.find((column) => column.columnName === 'dec');
216+
expect(decimalColumn?.typeDesc.types[0].primitiveEntry?.typeQualifiers).to.deep.equal({
217+
qualifiers: { precision: { i32Value: 10 }, scale: { i32Value: 2 } },
218+
});
214219
// TIMESTAMP: Phase-1 produces JS Date for arrow timestamps
215220
expect(row.ts).to.be.instanceOf(Date);
216221
expect((row.ts as Date).toISOString()).to.equal('2026-05-15T12:00:00.000Z');
@@ -221,6 +226,19 @@ describe('KernelOperationBackend — M0 datatype round-trip via napi → ArrowRe
221226
expect(await backend.hasMore()).to.equal(false);
222227
});
223228

229+
it('preserves decimal qualifiers for a schema-only result with zero scale', async () => {
230+
const schema = new Schema([new Field('amount', new Decimal(0, 38, 128), true)]);
231+
const backend = new KernelOperationBackend({
232+
statement: new StatementStub(ipcSchemaOnly(schema), []),
233+
context: new ClientContextStub(),
234+
});
235+
const metadata = await backend.getResultMetadata();
236+
expect(metadata.schema?.columns[0].typeDesc.types[0].primitiveEntry?.typeQualifiers).to.deep.equal({
237+
qualifiers: { precision: { i32Value: 38 }, scale: { i32Value: 0 } },
238+
});
239+
expect(await backend.hasMore()).to.equal(false);
240+
});
241+
224242
it('round-trips ARRAY / MAP / STRUCT via the converter Phase-2 JSON fallback', async () => {
225243
// ARRAY / MAP / STRUCT have two possible wire encodings in M0:
226244
// (a) native Arrow `List` / `Map` / `Struct` — Phase 1 produces plain

0 commit comments

Comments
 (0)