From d4a3d99075e272e91f105e6927bf5f5a03f006ca Mon Sep 17 00:00:00 2001 From: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Date: Thu, 6 Aug 2026 07:33:21 +0000 Subject: [PATCH] fix(node): handle deferred schema evidence --- nodejs/__test__/arrow.test.ts | 59 ++++++++- nodejs/lancedb/arrow.ts | 242 ++++++++++++++++++++++++++++------ 2 files changed, 263 insertions(+), 38 deletions(-) diff --git a/nodejs/__test__/arrow.test.ts b/nodejs/__test__/arrow.test.ts index 55aae754e..613959b5d 100644 --- a/nodejs/__test__/arrow.test.ts +++ b/nodejs/__test__/arrow.test.ts @@ -457,10 +457,67 @@ describe.each([arrow15, arrow16, arrow17, arrow18])( it("will reject mismatched inferred types across records", function () { expect(() => makeArrowTable([{ value: 1 }, { value: "two" }])).toThrow( - "Failed to infer schema for data. Previously inferred type Float64 but found Utf8 at row 1. Consider providing an explicit schema.", + "Failed to infer schema for data. Previously inferred type Float64 but found Utf8 for field value at row 1. Consider providing an explicit schema.", ); }); + it("will preserve null values without treating them as type mismatches", function () { + for (const records of [ + [{ vector: [1, 2, 3] }, { vector: null }], + [{ vector: null }, { vector: [1, 2, 3] }], + ]) { + const table = makeArrowTable(records); + + expect(table.numRows).toBe(2); + expect(table.getChild("vector")?.nullCount).toBe(1); + } + }); + + it("will preserve empty variable-size lists", function () { + for (const records of [ + [{ items: [1] }, { items: [] }], + [{ items: [] }, { items: [1] }], + ]) { + const table = makeArrowTable(records); + expect( + table + .getChild("items") + ?.toJSON() + .map((value) => value.toJSON()), + ).toEqual(records.map((record) => record.items)); + } + }); + + it("will reject empty fixed-size lists", function () { + expect(() => + makeArrowTable([{ vector: [1, 2, 3] }, { vector: [] }]), + ).toThrow( + "Failed to infer schema for data. Previously inferred type FixedSizeList[3] but found List[0] for field vector at row 1.", + ); + }); + + it("will reject inferred leaf and branch shape changes", function () { + expect(() => + makeArrowTable([{ value: 1 }, { value: { nested: 2 } }]), + ).toThrow( + "Failed to infer schema for data. Previously inferred type Float64 but found Struct for field value at row 1.", + ); + expect(() => + makeArrowTable([{ value: { nested: 1 } }, { value: 2 }]), + ).toThrow( + "Failed to infer schema for data. Previously inferred type Struct but found Float64 for field value at row 1.", + ); + }); + + it("will allow null values around inferred struct values", function () { + expect(() => + makeArrowTable([{ value: null }, { value: { nested: 2 } }]), + ).not.toThrow(); + expect(() => + makeArrowTable([{ value: { nested: 1 } }, { value: null }]), + ).not.toThrow(); + }); + it("will allow a schema to be provided", async function () { await checkTableCreation( async (records, _, schema) => diff --git a/nodejs/lancedb/arrow.ts b/nodejs/lancedb/arrow.ts index b0decd481..c53691f17 100644 --- a/nodejs/lancedb/arrow.ts +++ b/nodejs/lancedb/arrow.ts @@ -42,6 +42,7 @@ import { Utf8, Vector, makeVector as arrowMakeVector, + util as arrowUtil, vectorFromArray as badVectorFromArray, makeBuilder, makeData, @@ -455,17 +456,70 @@ export function makeArrowTable( return new ArrowTable(inferredSchema, finalColumns); } +const NO_TYPE_EVIDENCE = Symbol("no-type-evidence"); + +type NoTypeEvidence = { + kind: typeof NO_TYPE_EVIDENCE; + valueKind: "null" | "empty-list"; + row: number; +}; + +function getNoTypeEvidence( + value: unknown, + row: number, +): NoTypeEvidence | undefined { + if (value === null) { + return { kind: NO_TYPE_EVIDENCE, valueKind: "null", row }; + } + if (Array.isArray(value) && value.length === 0) { + return { kind: NO_TYPE_EVIDENCE, valueKind: "empty-list", row }; + } + return undefined; +} + +function isNoTypeEvidence( + value: DataType | NoTypeEvidence, +): value is NoTypeEvidence { + return "kind" in value && value.kind === NO_TYPE_EVIDENCE; +} + +function describeInferredType( + value: DataType | NoTypeEvidence | undefined, +): string { + if (value === undefined) { + return "an unsupported value"; + } + if (isNoTypeEvidence(value)) { + return value.valueKind === "null" ? "null" : "List[0]"; + } + return value.toString(); +} + +function schemaInferenceError( + path: string[], + row: number, + currentType: string, + newType: string, +): Error { + return new Error( + `Failed to infer schema for data. Previously inferred type ${currentType} ` + + `but found ${newType} for field ${path.join(".")} at row ${row}. ` + + "Consider providing an explicit schema.", + ); +} + function inferSchema( data: Array>, schema: Schema | undefined, opts: MakeArrowTableOptions, ): Schema { // We will collect all fields we see in the data. - const pathTree = new PathTree(); + const pathTree = new PathTree(); for (const [rowI, row] of data.entries()) { for (const [path, value] of rowPathsAndValues(row)) { - if (!pathTree.has(path)) { + const currentType = pathTree.get(path); + if (currentType === undefined) { // First time seeing this field. if (schema !== undefined) { const field = getFieldForPath(schema, path); @@ -474,25 +528,111 @@ function inferSchema( `Found field not in schema: ${path.join(".")} at row ${rowI}`, ); } else { - pathTree.set(path, field.type); + const conflict = pathTree.set(path, field.type); + if (conflict !== undefined) { + throw schemaInferenceError( + conflict.path, + rowI, + conflict.value instanceof PathTree + ? "Struct" + : describeInferredType(conflict.value), + "Struct", + ); + } } } else { const inferredType = inferType(value, path, opts); - if (inferredType === undefined) { + const noTypeEvidence = getNoTypeEvidence(value, rowI); + if (inferredType === undefined && noTypeEvidence === undefined) { throw new Error(`Failed to infer data type for field ${path.join( ".", )} at row ${rowI}. \ Consider providing an explicit schema.`); } - pathTree.set(path, inferredType); + + const conflict = pathTree.set( + path, + inferredType ?? (noTypeEvidence as NoTypeEvidence), + (existing) => + isNoTypeEvidence(existing) && existing.valueKind === "null", + ); + if (conflict !== undefined) { + throw schemaInferenceError( + conflict.path, + rowI, + conflict.value instanceof PathTree + ? "Struct" + : describeInferredType(conflict.value), + "Struct", + ); + } } } else if (schema === undefined) { - const currentType = pathTree.get(path); const newType = inferType(value, path, opts); - if (currentType?.toString() !== newType?.toString()) { - throw new Error( - `Failed to infer schema for data. Previously inferred type ${currentType} ` + - `but found ${newType} at row ${rowI}. Consider providing an explicit schema.`, + const noTypeEvidence = getNoTypeEvidence(value, rowI); + + if (currentType instanceof PathTree) { + if (noTypeEvidence?.valueKind === "null") { + continue; + } + throw schemaInferenceError( + path, + rowI, + "Struct", + describeInferredType(newType ?? noTypeEvidence), + ); + } + + if (isNoTypeEvidence(currentType)) { + if (newType !== undefined) { + if ( + currentType.valueKind === "empty-list" && + !DataType.isList(newType) + ) { + throw schemaInferenceError( + path, + rowI, + describeInferredType(currentType), + describeInferredType(newType), + ); + } + pathTree.set(path, newType); + } else if (noTypeEvidence !== undefined) { + if ( + currentType.valueKind === "null" && + noTypeEvidence.valueKind === "empty-list" + ) { + pathTree.set(path, noTypeEvidence); + } + } else { + throw schemaInferenceError( + path, + rowI, + describeInferredType(currentType), + describeInferredType(newType), + ); + } + } else if (newType !== undefined) { + if (!arrowUtil.compareTypes(currentType, newType)) { + throw schemaInferenceError( + path, + rowI, + describeInferredType(currentType), + describeInferredType(newType), + ); + } + } else if ( + noTypeEvidence?.valueKind !== "null" && + !( + noTypeEvidence?.valueKind === "empty-list" && + DataType.isList(currentType) + ) + ) { + throw schemaInferenceError( + path, + rowI, + describeInferredType(currentType), + describeInferredType(noTypeEvidence), ); } } @@ -500,12 +640,21 @@ function inferSchema( } if (schema === undefined) { - function fieldsFromPathTree(pathTree: PathTree): Field[] { + function fieldsFromPathTree( + pathTree: PathTree, + basePath: string[] = [], + ): Field[] { const fields = []; for (const [name, value] of pathTree.map.entries()) { if (value instanceof PathTree) { - const children = fieldsFromPathTree(value); + const children = fieldsFromPathTree(value, [...basePath, name]); fields.push(new Field(name, new Struct(children), true)); + } else if (isNoTypeEvidence(value)) { + throw new Error(`Failed to infer data type for field ${[ + ...basePath, + name, + ].join(".")} at row ${value.row}. \ + Consider providing an explicit schema.`); } else { fields.push(new Field(name, value, true)); } @@ -517,7 +666,7 @@ function inferSchema( } else { function takeMatchingFields( fields: Field[], - pathTree: PathTree, + pathTree: PathTree, ): Field[] { const outFields = []; for (const field of fields) { @@ -673,6 +822,11 @@ function inferType( } } +type PathConflict = { + path: string[]; + value: V | PathTree; +}; + class PathTree { map: Map>; @@ -684,35 +838,49 @@ class PathTree { } } } - has(path: string[]): boolean { - let ref: PathTree = this; + get(path: string[]): V | PathTree | undefined { + let ref: V | PathTree = this; for (const part of path) { - if (!(ref instanceof PathTree) || !ref.map.has(part)) { - return false; - } - ref = ref.map.get(part) as PathTree; - } - return true; - } - get(path: string[]): V | undefined { - let ref: PathTree = this; - for (const part of path) { - if (!(ref instanceof PathTree) || !ref.map.has(part)) { + if (!(ref instanceof PathTree)) { return undefined; } - ref = ref.map.get(part) as PathTree; - } - return ref as V; - } - set(path: string[], value: V): void { - let ref: PathTree = this; - for (const part of path.slice(0, path.length - 1)) { - if (!ref.map.has(part)) { - ref.map.set(part, new PathTree()); + const child = ref.map.get(part); + if (child === undefined) { + return undefined; } - ref = ref.map.get(part) as PathTree; + ref = child; } - ref.map.set(path[path.length - 1], value); + return ref; + } + set( + path: string[], + value: V, + canReplaceLeaf: (value: V) => boolean = () => false, + ): PathConflict | undefined { + let ref: PathTree = this; + for (const [index, part] of path.slice(0, path.length - 1).entries()) { + const child = ref.map.get(part); + if (child === undefined) { + const branch = new PathTree(); + ref.map.set(part, branch); + ref = branch; + } else if (child instanceof PathTree) { + ref = child; + } else if (canReplaceLeaf(child)) { + const branch = new PathTree(); + ref.map.set(part, branch); + ref = branch; + } else { + return { path: path.slice(0, index + 1), value: child }; + } + } + const name = path[path.length - 1]; + const current = ref.map.get(name); + if (current instanceof PathTree) { + return { path, value: current }; + } + ref.map.set(name, value); + return undefined; } }