Skip to content

Commit fc7ff6c

Browse files
fix(ts): return undefined for empty/incomplete string columns (#1469)
Split from #1462. - `decodeString` now returns `undefined` for empty columns, and the dictionary/FSST decoders throw a named error when a column's companion streams are incomplete instead of constructing a half-built vector. - `propertyDecoder` keeps its existing `null` contract via `?? null`. Adds coverage for the empty/incomplete cases
1 parent c9c3970 commit fc7ff6c

6 files changed

Lines changed: 180 additions & 70 deletions

File tree

ts/src/decoding/propertyDecoder.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function decodePropertyColumn(
3636
numStreams: number,
3737
numFeatures: number,
3838
propertyColumnNames?: Set<string>,
39-
): Vector | Vector[] {
39+
): Vector | Vector[] | null {
4040
if (columnMetadata.type === "scalarType") {
4141
if (propertyColumnNames && !propertyColumnNames.has(columnMetadata.name)) {
4242
skipColumn(numStreams, data, offset);
@@ -68,7 +68,7 @@ function decodeScalarPropertyColumn(
6868
column: ScalarColumn,
6969
columnMetadata: Column,
7070
) {
71-
let nullabilityBuffer: BitVector = null;
71+
let nullabilityBuffer: BitVector | undefined;
7272
if (numStreams === 0) {
7373
return null;
7474
}
@@ -91,7 +91,7 @@ function decodeScalarPropertyColumn(
9191
case ScalarType.STRING: {
9292
// In embedded format: numStreams includes nullability stream if column is nullable
9393
const stringDataStreams = columnMetadata.nullable ? numStreams - 1 : numStreams;
94-
return decodeString(columnMetadata.name, data, offset, stringDataStreams, nullabilityBuffer);
94+
return decodeString(columnMetadata.name, data, offset, stringDataStreams, nullabilityBuffer) ?? null;
9595
}
9696
case ScalarType.BOOLEAN:
9797
return decodeBooleanColumn(data, offset, columnMetadata, numFeatures, sizeOrNullabilityBuffer);

ts/src/decoding/stringDecoder.spec.ts

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ import {
1313
import { StringFlatVector } from "../vector/flat/stringFlatVector";
1414
import { StringDictionaryVector } from "../vector/dictionary/stringDictionaryVector";
1515
import { StringFsstDictionaryVector } from "../vector/fsst-dictionary/stringFsstDictionaryVector";
16-
import { ScalarType } from "../metadata/tileset/tilesetMetadata";
16+
import { type Column, ColumnScope, ScalarType } from "../metadata/tileset/tilesetMetadata";
1717
import { PhysicalStreamType } from "../metadata/tile/physicalStreamType";
1818
import { LengthType } from "../metadata/tile/lengthType";
19+
import { DictionaryType } from "../metadata/tile/dictionaryType";
1920

2021
describe("decodeString - Plain String Decoder", () => {
2122
it("should decode plain strings with simple ASCII values", () => {
@@ -175,20 +176,20 @@ describe("decodeString - FSST Dictionary Decoder (Basic Coverage)", () => {
175176
});
176177

177178
describe("decodeString - Empty Column Edge Cases", () => {
178-
it("should handle empty column with numStreams = 0 (returns null)", () => {
179+
it("should handle empty column with numStreams = 0 (returns undefined)", () => {
179180
const fullStream = new Uint8Array([]);
180181
const offset = new IntWrapper(0);
181182
const result = decodeString("testColumn", fullStream, offset, 0);
182-
expect(result).toBeNull();
183+
expect(result).toBeUndefined();
183184
});
184185

185-
it("should handle column with all zero-length streams (returns null)", () => {
186+
it("should handle column with all zero-length streams (returns undefined)", () => {
186187
const emptyStream = createStream(PhysicalStreamType.LENGTH, new Uint8Array([]), {
187188
logical: { lengthType: LengthType.VAR_BINARY },
188189
});
189190
const offset = new IntWrapper(0);
190191
const result = decodeString("testColumn", emptyStream, offset, 1);
191-
expect(result).toBeNull();
192+
expect(result).toBeUndefined();
192193
});
193194

194195
it("should handle single value plain string column", () => {
@@ -210,6 +211,28 @@ describe("decodeString - Empty Column Edge Cases", () => {
210211
});
211212
});
212213

214+
describe("decodeString - incomplete column streams throw", () => {
215+
it("throws when an FSST symbol table is present but its companion streams are missing", () => {
216+
const symbolTableOnly = createStream(PhysicalStreamType.DATA, new Uint8Array([1, 2, 3]), {
217+
logical: { dictionaryType: DictionaryType.FSST },
218+
});
219+
const offset = new IntWrapper(0);
220+
expect(() => decodeString("fsstColumn", symbolTableOnly, offset, 1)).toThrow(
221+
'Incomplete FSST dictionary string column "fsstColumn"',
222+
);
223+
});
224+
225+
it("throws when a dictionary stream is present but offsets/lengths are missing", () => {
226+
const dictionaryDataOnly = createStream(PhysicalStreamType.DATA, new Uint8Array([1, 2, 3]), {
227+
logical: { dictionaryType: DictionaryType.SINGLE },
228+
});
229+
const offset = new IntWrapper(0);
230+
expect(() => decodeString("dictColumn", dictionaryDataOnly, offset, 1)).toThrow(
231+
'Incomplete dictionary string column "dictColumn"',
232+
);
233+
});
234+
});
235+
213236
describe("decodeString - Integration Tests", () => {
214237
it("should correctly track offset through multiple streams", () => {
215238
const strings = ["hello", "world"];
@@ -503,6 +526,46 @@ describe("decodeSharedDictionary", () => {
503526
}).toThrow("Currently only scalar string fields are implemented for a struct.");
504527
});
505528

529+
it("should throw when the column is not a complex (struct) column", () => {
530+
const { lengthStream, dataStream } = encodeSharedDictionary(["value"]);
531+
const complete = concatenateBuffers(lengthStream, dataStream);
532+
const scalarColumn: Column = {
533+
name: "notAStruct",
534+
nullable: false,
535+
columnScope: ColumnScope.FEATURE,
536+
type: "scalarType",
537+
scalarType: { longID: false, type: "physicalType", physicalType: ScalarType.STRING },
538+
};
539+
540+
expect(() => {
541+
decodeSharedDictionary(complete, new IntWrapper(0), scalarColumn);
542+
}).toThrow("Shared dictionary column notAStruct must be a complex (struct) column.");
543+
});
544+
545+
it("should throw when the shared dictionary offsets are missing", () => {
546+
// Only the dictionary data stream is present, so the dictionary offset buffer is never decoded.
547+
const { dataStream } = encodeSharedDictionary(["value"]);
548+
const columnMetadata = createColumnMetadataForStruct("incomplete:", [{ name: "field1" }]);
549+
550+
expect(() => {
551+
decodeSharedDictionary(dataStream, new IntWrapper(0), columnMetadata);
552+
}).toThrow('Incomplete shared dictionary for column "incomplete:"');
553+
});
554+
555+
it("should throw when an FSST shared dictionary is missing its symbol offsets", () => {
556+
// Provide the dictionary length + data and the FSST symbol table, but omit the symbol length stream
557+
// so the symbol offset buffer is never decoded.
558+
const { lengthStream, dataStream, symbolDataStream } = encodeSharedDictionary(["value"], { useFsst: true });
559+
if (!symbolDataStream) throw new Error("test setup: expected an FSST symbol data stream");
560+
const fieldStreams = encodeStructField([0], [true]);
561+
const complete = concatenateBuffers(lengthStream, symbolDataStream, dataStream, fieldStreams);
562+
const columnMetadata = createColumnMetadataForStruct("fsst:", [{ name: "value" }]);
563+
564+
expect(() => {
565+
decodeSharedDictionary(complete, new IntWrapper(0), columnMetadata);
566+
}).toThrow('Incomplete shared FSST dictionary for column "fsst:value"');
567+
});
568+
506569
it("should throw error for mismatched nullability and numStreams", () => {
507570
const dictionaryStrings = ["value"];
508571
const { lengthStream, dataStream } = encodeSharedDictionary(dictionaryStrings);

ts/src/decoding/stringDecoder.ts

Lines changed: 68 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ export function decodeString(
2222
offset: IntWrapper,
2323
numStreams: number,
2424
bitVector?: BitVector,
25-
): Vector {
26-
let dictionaryLengthStream: Uint32Array = null;
27-
let offsetStream: Uint32Array = null;
28-
let dictionaryStream: Uint8Array = null;
29-
let symbolLengthStream: Uint32Array = null;
30-
let symbolTableStream: Uint8Array = null;
31-
let nullabilityBuffer: BitVector = bitVector ?? null;
32-
let plainLengthStream: Uint32Array = null;
33-
let plainDataStream: Uint8Array = null;
25+
): Vector | undefined {
26+
let dictionaryLengthStream: Uint32Array | undefined;
27+
let offsetStream: Uint32Array | undefined;
28+
let dictionaryStream: Uint8Array | undefined;
29+
let symbolLengthStream: Uint32Array | undefined;
30+
let symbolTableStream: Uint8Array | undefined;
31+
let nullabilityBuffer: BitVector | undefined = bitVector;
32+
let plainLengthStream: Uint32Array | undefined;
33+
let plainDataStream: Uint8Array | undefined;
3434

3535
for (let i = 0; i < numStreams; i++) {
3636
const streamMetadata = decodeStreamMetadata(data, offset);
@@ -91,15 +91,18 @@ export function decodeString(
9191

9292
function decodeFsstDictionaryVector(
9393
name: string,
94-
symbolTableStream: Uint8Array | null,
95-
offsetStream: Uint32Array | null,
96-
dictionaryLengthStream: Uint32Array | null,
97-
dictionaryStream: Uint8Array | null,
98-
symbolLengthStream: Uint32Array | null,
99-
nullabilityBuffer: BitVector | null,
100-
): Vector | null {
94+
symbolTableStream: Uint8Array | undefined,
95+
offsetStream: Uint32Array | undefined,
96+
dictionaryLengthStream: Uint32Array | undefined,
97+
dictionaryStream: Uint8Array | undefined,
98+
symbolLengthStream: Uint32Array | undefined,
99+
nullabilityBuffer: BitVector | undefined,
100+
): Vector | undefined {
101101
if (!symbolTableStream) {
102-
return null;
102+
return undefined;
103+
}
104+
if (!offsetStream || !dictionaryLengthStream || !dictionaryStream || !symbolLengthStream) {
105+
throw new Error(`Incomplete FSST dictionary string column "${name}"`);
103106
}
104107
return new StringFsstDictionaryVector(
105108
name,
@@ -114,13 +117,16 @@ function decodeFsstDictionaryVector(
114117

115118
function decodeDictionaryVector(
116119
name: string,
117-
dictionaryStream: Uint8Array | null,
118-
offsetStream: Uint32Array | null,
119-
dictionaryLengthStream: Uint32Array | null,
120-
nullabilityBuffer: BitVector | null,
121-
): Vector | null {
120+
dictionaryStream: Uint8Array | undefined,
121+
offsetStream: Uint32Array | undefined,
122+
dictionaryLengthStream: Uint32Array | undefined,
123+
nullabilityBuffer: BitVector | undefined,
124+
): Vector | undefined {
122125
if (!dictionaryStream) {
123-
return null;
126+
return undefined;
127+
}
128+
if (!offsetStream || !dictionaryLengthStream) {
129+
throw new Error(`Incomplete dictionary string column "${name}"`);
124130
}
125131
return nullabilityBuffer
126132
? new StringDictionaryVector(name, offsetStream, dictionaryLengthStream, dictionaryStream, nullabilityBuffer)
@@ -129,13 +135,13 @@ function decodeDictionaryVector(
129135

130136
function decodePlainStringVector(
131137
name: string,
132-
plainLengthStream: Uint32Array | null,
133-
plainDataStream: Uint8Array | null,
134-
offsetStream: Uint32Array | null,
135-
nullabilityBuffer: BitVector | null,
136-
): Vector | null {
138+
plainLengthStream: Uint32Array | undefined,
139+
plainDataStream: Uint8Array | undefined,
140+
offsetStream: Uint32Array | undefined,
141+
nullabilityBuffer: BitVector | undefined,
142+
): Vector | undefined {
137143
if (!plainLengthStream || !plainDataStream) {
138-
return null;
144+
return undefined;
139145
}
140146

141147
if (offsetStream) {
@@ -174,10 +180,10 @@ export function decodeSharedDictionary(
174180
column: Column,
175181
propertyColumnNames?: Set<string>,
176182
): Vector[] {
177-
let dictionaryOffsetBuffer: Uint32Array = null;
178-
let dictionaryBuffer: Uint8Array = null;
179-
let symbolOffsetBuffer: Uint32Array = null;
180-
let symbolTableBuffer: Uint8Array = null;
183+
let dictionaryOffsetBuffer: Uint32Array | undefined;
184+
let dictionaryBuffer: Uint8Array | undefined;
185+
let symbolOffsetBuffer: Uint32Array | undefined;
186+
let symbolTableBuffer: Uint8Array | undefined;
181187

182188
let dictionaryStreamDecoded = false;
183189
while (!dictionaryStreamDecoded) {
@@ -205,6 +211,12 @@ export function decodeSharedDictionary(
205211
}
206212
}
207213

214+
if (column.type !== "complexType") {
215+
throw new Error(`Shared dictionary column ${column.name} must be a complex (struct) column.`);
216+
}
217+
if (!dictionaryOffsetBuffer || !dictionaryBuffer) {
218+
throw new Error(`Incomplete shared dictionary for column "${column.name}"`);
219+
}
208220
const childFields = column.complexType.children;
209221
const stringDictionaryVectors = [];
210222
/** Shared by every FSST child-column vector in this SharedDict and populated on first access. */
@@ -255,24 +267,29 @@ export function decodeSharedDictionary(
255267
presentStreamBitVector,
256268
);
257269

258-
stringDictionaryVectors[i++] = symbolTableBuffer
259-
? new StringFsstDictionaryVector(
260-
columnName,
261-
offsetStream,
262-
dictionaryOffsetBuffer,
263-
dictionaryBuffer,
264-
symbolOffsetBuffer,
265-
symbolTableBuffer,
266-
presentStreamBitVector,
267-
sharedDictionaryCache,
268-
)
269-
: new StringDictionaryVector(
270-
columnName,
271-
offsetStream,
272-
dictionaryOffsetBuffer,
273-
dictionaryBuffer,
274-
presentStreamBitVector,
275-
);
270+
if (symbolTableBuffer) {
271+
if (!symbolOffsetBuffer) {
272+
throw new Error(`Incomplete shared FSST dictionary for column "${columnName}"`);
273+
}
274+
stringDictionaryVectors[i++] = new StringFsstDictionaryVector(
275+
columnName,
276+
offsetStream,
277+
dictionaryOffsetBuffer,
278+
dictionaryBuffer,
279+
symbolOffsetBuffer,
280+
symbolTableBuffer,
281+
presentStreamBitVector,
282+
sharedDictionaryCache,
283+
);
284+
} else {
285+
stringDictionaryVectors[i++] = new StringDictionaryVector(
286+
columnName,
287+
offsetStream,
288+
dictionaryOffsetBuffer,
289+
dictionaryBuffer,
290+
presentStreamBitVector,
291+
);
292+
}
276293
}
277294

278295
return stringDictionaryVectors;

ts/src/vector/featureTable.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { describe, it, expect } from "vitest";
2+
import FeatureTable from "./featureTable";
3+
import type Vector from "./vector";
4+
import { encodePointGeometryVector } from "../encoding/constGeometryVectorEncoder";
5+
6+
const geometryVector = encodePointGeometryVector(1, 2);
7+
const propertyVector = (name: string): Vector => ({ name }) as unknown as Vector;
8+
9+
describe("FeatureTable", () => {
10+
it("throws when constructed without a layer name", () => {
11+
expect(() => new FeatureTable("", geometryVector)).toThrow("Missing layer name");
12+
});
13+
14+
it("returns an empty array when no property vectors are provided", () => {
15+
const table = new FeatureTable("layer", geometryVector);
16+
expect(table.propertyVectors).toEqual([]);
17+
});
18+
19+
it("looks up property vectors by name and returns undefined for unknown names", () => {
20+
const a = propertyVector("a");
21+
const b = propertyVector("b");
22+
const table = new FeatureTable("layer", geometryVector, undefined, [a, b]);
23+
24+
expect(table.getPropertyVector("a")).toBe(a);
25+
expect(table.getPropertyVector("b")).toBe(b);
26+
expect(table.getPropertyVector("missing")).toBeUndefined();
27+
});
28+
});

ts/src/vector/featureTable.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ import { Int32ConstVector } from "./constant/int32ConstVector";
88
import type { GpuVector } from "./geometry/gpuVector";
99

1010
export interface Feature {
11-
id: number | bigint;
11+
id: number | bigint | undefined;
1212
geometry: Geometry;
1313
properties: { [key: string]: unknown };
1414
}
1515

1616
export default class FeatureTable {
17-
private propertyVectorsMap: Map<string, Vector>;
17+
private propertyVectorsMap?: Map<string, Vector>;
1818

1919
constructor(
2020
private readonly _name: string,
@@ -32,7 +32,7 @@ export default class FeatureTable {
3232
return this._name;
3333
}
3434

35-
get idVector(): IdVector {
35+
get idVector(): IdVector | undefined {
3636
return this._idVector;
3737
}
3838

@@ -41,12 +41,12 @@ export default class FeatureTable {
4141
}
4242

4343
get propertyVectors(): Vector[] {
44-
return this._propertyVectors;
44+
return this._propertyVectors ?? [];
4545
}
4646

47-
getPropertyVector(name: string): Vector {
47+
getPropertyVector(name: string): Vector | undefined {
4848
if (!this.propertyVectorsMap) {
49-
this.propertyVectorsMap = new Map(this._propertyVectors.map((vector) => [vector.name, vector]));
49+
this.propertyVectorsMap = new Map(this.propertyVectors.map((vector) => [vector.name, vector]));
5050
}
5151

5252
return this.propertyVectorsMap.get(name);
@@ -68,10 +68,12 @@ export default class FeatureTable {
6868
const geometries = this.geometryVector.getGeometries();
6969

7070
for (let i = 0; i < this.numFeatures; i++) {
71-
let id;
71+
let id: number | bigint | undefined;
7272
if (this.idVector) {
7373
const idValue = this.idVector.getValue(i);
74-
id = this.containsMaxSafeIntegerValues(this.idVector) && idValue !== null ? Number(idValue) : idValue;
74+
if (idValue !== null) {
75+
id = this.containsMaxSafeIntegerValues(this.idVector) ? Number(idValue) : idValue;
76+
}
7577
}
7678
const geometry = {
7779
coordinates: geometries[i],

0 commit comments

Comments
 (0)