Skip to content

Commit 2626667

Browse files
authored
fix(firestore-bigquery-export): accept ISO 8601 string partition values (#2813)
* fix(firestore-bigquery-export): accept ISO 8601 string partition values Restores 0.2.x behavior for Firestore string partition fields. The 0.3.0 refactor of PartitionValueConverter narrowed accepted inputs to Firestore Timestamp / timestamp-like / Date, silently coercing strings (including ISO 8601 dates such as "2026-01-01") to NULL and landing rows in the __NULL__ partition. PartitionValueConverter.convert now parses strings via new Date(value). Unparseable strings still return null and trigger the existing warning. Adds a defensive try/catch around the BigQuery formatter switch so any serialization failure degrades to null + warn rather than crashing the row write. Fixes #2803 * fix(firestore-bigquery-export): explicit default in partition value switch Adds default: return null to PartitionValueConverter.convert. fieldType is typed as a strict union, so exhaustiveness is already enforced at compile time, but the value comes from external config — a runtime mismatch falls through cleanly to null instead of returning undefined. Per gemini-code-assist review on #2813. * chore(firestore-bigquery-export): bump version to 0.3.2 * fix(firestore-bigquery-export): strict ISO 8601 validation for partition strings JavaScript's Date parser is too permissive for partition value validation: new Date("2024-02-30") -> 2024-03-01 (silent month overflow) new Date("2024-01") -> 2024-01-01 (silent partial-date fill) new Date("1") -> 2001-01-01 (bare numeric as year) new Date("2023-02-29") -> 2023-03-01 (non-leap-year overflow) new Date("2024-01-15T10:30") -> engine-dependent local-time interpretation Replaces the loose new Date() check with a strict YYYY-MM-DD prefix regex that requires an explicit timezone designator (Z or +/-HH:MM) when a time component is present, plus a calendar validator built via setUTCFullYear that rejects calendar-invalid components like Feb 30, non-leap-year Feb 29, month 13, and day 32. Per CorieW review on #2813. * fix(firestore-bigquery-export): reject year 0 in partition strings BigQuery DATE / DATETIME / TIMESTAMP all reject year 0 — the supported range is 0001-01-01 to 9999-12-31. The previous strict validator passed "0000-01-01" through (setUTCFullYear(0, 0, 1) yields year 0, matching input components) but BigQuery rejects it server-side, causing the same NULL-partition symptom this PR is fixing. Reject yearN < 1 client-side so the user gets a clear warning log. * test(firestore-bigquery-export): cover partition string edge cases Adds 4 cases to PartitionValueConverter TIMESTAMP suite: - out-of-range hour ("25:00:00Z") -> null - out-of-range minute ("23:60:00Z") -> null - timezone offset without colon ("+0800") -> accepted - fractional seconds beyond millisecond precision -> accepted The first two pin the implicit time-validation behavior (regex matches H/M/S as 2-digit pairs, then new Date() rejects out-of-range values). The last two document accepted RFC 3339 variants. * test(firestore-bigquery-export): pin DATETIME canonical output form Adds a strict-equality test that asserts the DATETIME converter outputs "2024-01-15 10:30:00.000" (space separator, no Z) when fed an ISO 8601 string with a Z suffix. The existing tests only checked that the result contained "2024-01-15", which left the canonical-form contract unpinned. @google-cloud/bigquery's BigQuery.datetime() already normalises Z-suffixed input to BigQuery's canonical DATETIME form, so the production code is correct. This test guards against future regressions. * test(firestore-bigquery-export): cover space-separated partition strings The regex in PartitionValueConverter accepts both "T" and a literal space as the date/time separator (RFC 3339 allows either), but no test covered the space form. Pinned so the contract is explicit. * fix(firestore-bigquery-export): reject hour 24 in partition strings ISO 8601 allows "24:00:00" as an alias for next-day midnight, and JS Date parses it that way. The new string parsing path in 0.3.2 would therefore silently roll inputs like "2024-01-15T24:00:00Z" forward to the "2024-01-16" partition. That is a regression vs 0.2.x, where the raw string was passed straight to BigQuery and BigQuery rejected hour 24 outright, surfacing the bad row instead of misfiling it. Reject hour 24 explicitly so the caller logs firestoreTimePartitionFieldError and the row lands in the __NULL__ partition (loud failure, matching the calendar-validator philosophy already applied to Feb 30 etc.). Minute and second out-of-range values are still caught by new Date() returning Invalid Date in the existing check below. Adds a test pinning the hour 24 rejection.
1 parent a675f5c commit 2626667

4 files changed

Lines changed: 221 additions & 10 deletions

File tree

firestore-bigquery-export/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## Version 0.3.2
2+
3+
fix: restore acceptance of ISO 8601 date/datetime strings as partition field values, regression introduced in 0.3.0 (#2803)
4+
15
## Version 0.3.1
26

37
chore: bump dependencies

firestore-bigquery-export/extension.yaml

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

1515
name: firestore-bigquery-export
16-
version: 0.3.1
16+
version: 0.3.2
1717
specVersion: v1beta
1818

1919
displayName: Stream Firestore to BigQuery

firestore-bigquery-export/firestore-bigquery-change-tracker/src/__tests__/bigquery/partitioning/converter.test.ts

Lines changed: 155 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,105 @@ describe("PartitionValueConverter", () => {
4242
expect(result).toBeNull();
4343
});
4444

45-
test("returns null for string", () => {
45+
test("converts ISO 8601 datetime string to BigQuery timestamp string", () => {
46+
const result = converter.convert("2024-01-15T10:30:00Z");
47+
expect(result).toContain("2024-01-15");
48+
});
49+
50+
test("converts ISO 8601 date-only string to BigQuery timestamp string", () => {
4651
const result = converter.convert("2024-01-15");
47-
expect(result).toBeNull();
52+
expect(result).toContain("2024-01-15");
53+
});
54+
55+
test("returns null for unparseable string", () => {
56+
expect(converter.convert("not-a-date")).toBeNull();
57+
});
58+
59+
test("returns null for empty string", () => {
60+
expect(converter.convert("")).toBeNull();
61+
});
62+
63+
test("returns null for partial date (year-month only)", () => {
64+
expect(converter.convert("2024-01")).toBeNull();
65+
});
66+
67+
test("returns null for partial date (year only)", () => {
68+
expect(converter.convert("2024")).toBeNull();
69+
});
70+
71+
test("returns null for bare numeric string", () => {
72+
expect(converter.convert("1")).toBeNull();
73+
});
74+
75+
test("returns null for calendar-invalid date (Feb 30)", () => {
76+
expect(converter.convert("2024-02-30")).toBeNull();
77+
});
78+
79+
test("returns null for non-leap-year Feb 29", () => {
80+
expect(converter.convert("2023-02-29")).toBeNull();
81+
});
82+
83+
test("accepts leap-year Feb 29", () => {
84+
const result = converter.convert("2024-02-29");
85+
expect(result).toContain("2024-02-29");
86+
});
87+
88+
test("returns null for out-of-range month", () => {
89+
expect(converter.convert("2024-13-01")).toBeNull();
90+
});
91+
92+
test("returns null for out-of-range day", () => {
93+
expect(converter.convert("2024-01-32")).toBeNull();
94+
});
95+
96+
test("returns null for year 0 (outside BigQuery DATE range)", () => {
97+
expect(converter.convert("0000-01-01")).toBeNull();
98+
});
99+
100+
test("accepts year 0001 (BigQuery DATE minimum)", () => {
101+
const result = converter.convert("0001-01-01");
102+
expect(result).toContain("0001-01-01");
103+
});
104+
105+
test("accepts year 9999 (BigQuery DATE maximum)", () => {
106+
const result = converter.convert("9999-12-31");
107+
expect(result).toContain("9999-12-31");
108+
});
109+
110+
test("returns null for datetime without timezone", () => {
111+
expect(converter.convert("2024-01-15T10:30:00")).toBeNull();
112+
});
113+
114+
test("returns null for out-of-range hour", () => {
115+
expect(converter.convert("2024-01-15T25:00:00Z")).toBeNull();
116+
});
117+
118+
test("returns null for hour 24 (avoids silent next-day shift)", () => {
119+
// ISO 8601 allows 24:00:00 as end-of-day, equivalent to next day 00:00.
120+
// JS Date parses it as such and rolls forward, which would silently
121+
// misfile the row into the next-day partition. 0.2.x passed the raw
122+
// string to BigQuery, which rejected hour=24 outright. Reject here to
123+
// match the loud-failure behavior rather than silent misfiling.
124+
expect(converter.convert("2024-01-15T24:00:00Z")).toBeNull();
125+
});
126+
127+
test("returns null for out-of-range minute", () => {
128+
expect(converter.convert("2024-01-15T23:60:00Z")).toBeNull();
129+
});
130+
131+
test("accepts timezone offset without colon", () => {
132+
const result = converter.convert("2024-01-15T10:30:00+0800");
133+
expect(result).toContain("2024-01-15");
134+
});
135+
136+
test("accepts fractional seconds beyond millisecond precision", () => {
137+
const result = converter.convert("2024-01-15T10:30:00.123456Z");
138+
expect(result).toContain("2024-01-15");
139+
});
140+
141+
test("accepts space separator between date and time (RFC 3339 alt form)", () => {
142+
const result = converter.convert("2024-01-15 10:30:00Z");
143+
expect(result).toContain("2024-01-15");
48144
});
49145

50146
test("returns null for null", () => {
@@ -104,6 +200,32 @@ describe("PartitionValueConverter", () => {
104200
const result = converter.convert(date);
105201
expect(result).toBe("2024-01-15");
106202
});
203+
204+
test("converts ISO 8601 date-only string to BigQuery date string", () => {
205+
const result = converter.convert("2024-01-15");
206+
expect(result).toBe("2024-01-15");
207+
});
208+
209+
test("converts ISO 8601 datetime string to BigQuery date string", () => {
210+
const result = converter.convert("2024-01-15T10:30:00Z");
211+
expect(result).toBe("2024-01-15");
212+
});
213+
214+
test("uses UTC date component for timezone-suffixed datetime string", () => {
215+
// 2024-01-15T22:00:00-08:00 == 2024-01-16T06:00:00Z. The DATE column
216+
// takes the UTC date component, matching how Firestore Timestamps are
217+
// handled. Pinned so future changes to this contract are explicit.
218+
const result = converter.convert("2024-01-15T22:00:00-08:00");
219+
expect(result).toBe("2024-01-16");
220+
});
221+
222+
test("returns null for unparseable string", () => {
223+
expect(converter.convert("not-a-date")).toBeNull();
224+
});
225+
226+
test("returns null for empty string", () => {
227+
expect(converter.convert("")).toBeNull();
228+
});
107229
});
108230

109231
describe("convert with DATETIME type", () => {
@@ -134,5 +256,36 @@ describe("PartitionValueConverter", () => {
134256
expect(result).toBeDefined();
135257
expect(result).toContain("2024-01-15");
136258
});
259+
260+
test("converts ISO 8601 datetime string to BigQuery datetime string", () => {
261+
const result = converter.convert("2024-01-15T10:30:00Z");
262+
expect(result).toBeDefined();
263+
expect(result).toContain("2024-01-15");
264+
});
265+
266+
test("converts ISO 8601 date-only string to BigQuery datetime string", () => {
267+
const result = converter.convert("2024-01-15");
268+
expect(result).toBeDefined();
269+
expect(result).toContain("2024-01-15");
270+
});
271+
272+
test("DATETIME output uses BigQuery canonical form (no Z, space separator)", () => {
273+
// BigQuery DATETIME columns reject the 'Z' timezone suffix and require
274+
// a space (not 'T') between date and time. @google-cloud/bigquery's
275+
// BigQuery.datetime() helper already normalises ISO 8601 input to this
276+
// canonical form, so feeding it date.toISOString() (which always ends
277+
// in 'Z') is safe. Pinned so the contract does not silently regress.
278+
expect(converter.convert("2024-01-15T10:30:00Z")).toBe(
279+
"2024-01-15 10:30:00.000"
280+
);
281+
});
282+
283+
test("returns null for unparseable string", () => {
284+
expect(converter.convert("not-a-date")).toBeNull();
285+
});
286+
287+
test("returns null for empty string", () => {
288+
expect(converter.convert("")).toBeNull();
289+
});
137290
});
138291
});

firestore-bigquery-export/firestore-bigquery-change-tracker/src/bigquery/partitioning/converter.ts

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,71 @@ export class PartitionValueConverter {
2727
).toDate();
2828
} else if (value instanceof Date && !isNaN(value.getTime())) {
2929
date = value;
30+
} else if (typeof value === "string") {
31+
// Strict ISO 8601 / RFC 3339: YYYY-MM-DD, optionally followed by T or
32+
// space-separated HH:MM[:SS[.ffffff]] and a required timezone designator
33+
// when the time component is present. JS Date parsing alone is too
34+
// permissive — it silently normalizes invalid inputs (e.g. "2024-02-30"
35+
// → "2024-03-01"), accepts partial dates ("2024-01"), and reads bare
36+
// numerics as years ("1" → "2001-01-01"). Reject all of those.
37+
const m = value.match(
38+
/^(\d{4})-(\d{2})-(\d{2})(?:[T ](\d{2}):(\d{2})(?::(\d{2})(?:\.\d+)?)?(Z|[+-]\d{2}:?\d{2}))?$/
39+
);
40+
if (!m) {
41+
return null;
42+
}
43+
const yearN = Number(m[1]);
44+
const monthN = Number(m[2]);
45+
const dayN = Number(m[3]);
46+
// BigQuery DATE / DATETIME / TIMESTAMP all reject year 0 — the supported
47+
// range is 0001-01-01 to 9999-12-31. Reject client-side so the row gets
48+
// a clear warning instead of a server-side insert error.
49+
if (yearN < 1) {
50+
return null;
51+
}
52+
// Reject calendar-invalid components (Feb 30, non-leap Feb 29, etc.).
53+
// setUTCFullYear avoids the legacy 2-digit-year quirk of Date.UTC().
54+
const validator = new Date(0);
55+
validator.setUTCFullYear(yearN, monthN - 1, dayN);
56+
if (
57+
validator.getUTCFullYear() !== yearN ||
58+
validator.getUTCMonth() + 1 !== monthN ||
59+
validator.getUTCDate() !== dayN
60+
) {
61+
return null;
62+
}
63+
// Reject hour 24 (ISO 8601 allows it as end-of-day, but JS Date and the
64+
// pre-0.3.0 string passthrough both treat it differently: JS rolls to
65+
// next day, BigQuery DATETIME rejects the row outright. Rather than
66+
// silently misfile the row into the next-day partition, reject here so
67+
// the caller logs firestoreTimePartitionFieldError and the row lands in
68+
// __NULL__. Minute and second out-of-range values are caught by
69+
// new Date() returning Invalid Date below.
70+
if (m[4] !== undefined && Number(m[4]) > 23) {
71+
return null;
72+
}
73+
const parsed = new Date(value);
74+
if (isNaN(parsed.getTime())) {
75+
return null;
76+
}
77+
date = parsed;
3078
} else {
3179
return null;
3280
}
3381

34-
switch (this.fieldType) {
35-
case "DATETIME":
36-
return BigQuery.datetime(date.toISOString()).value;
37-
case "DATE":
38-
return BigQuery.date(date.toISOString().substring(0, 10)).value;
39-
case "TIMESTAMP":
40-
return BigQuery.timestamp(date).value;
82+
try {
83+
switch (this.fieldType) {
84+
case "DATETIME":
85+
return BigQuery.datetime(date.toISOString()).value;
86+
case "DATE":
87+
return BigQuery.date(date.toISOString().substring(0, 10)).value;
88+
case "TIMESTAMP":
89+
return BigQuery.timestamp(date).value;
90+
default:
91+
return null;
92+
}
93+
} catch {
94+
return null;
4195
}
4296
}
4397
}

0 commit comments

Comments
 (0)