Skip to content

Commit a94fed9

Browse files
authored
Merge pull request #141 from tinybirdco/fix/low-cardinality-nullable-double-wrap
fix: don't set nullable modifier when nullable is embedded in LowCardinality type string
2 parents ed46c68 + 6c35e51 commit a94fed9

3 files changed

Lines changed: 26 additions & 5 deletions

File tree

src/generator/datasource.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ describe('Datasource Generator', () => {
142142
expect(result.content).toContain('country LowCardinality(String)');
143143
});
144144

145-
it('formats LowCardinality(Nullable) correctly', () => {
145+
it('formats LowCardinality(Nullable) correctly with .lowCardinality().nullable()', () => {
146146
const ds = defineDatasource('test_ds', {
147147
schema: {
148148
country: t.string().lowCardinality().nullable(),
@@ -151,6 +151,19 @@ describe('Datasource Generator', () => {
151151

152152
const result = generateDatasource(ds);
153153
expect(result.content).toContain('country LowCardinality(Nullable(String))');
154+
expect(result.content).not.toContain('Nullable(LowCardinality');
155+
});
156+
157+
it('formats LowCardinality(Nullable) correctly with .nullable().lowCardinality()', () => {
158+
const ds = defineDatasource('test_ds', {
159+
schema: {
160+
country: t.string().nullable().lowCardinality(),
161+
},
162+
});
163+
164+
const result = generateDatasource(ds);
165+
expect(result.content).toContain('country LowCardinality(Nullable(String))');
166+
expect(result.content).not.toContain('Nullable(LowCardinality');
154167
});
155168

156169
it('includes default values', () => {

src/schema/types.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,18 @@ describe("Type Validators (t.*)", () => {
8787
expect(type._tinybirdType).toBe("LowCardinality(Nullable(String))");
8888
});
8989

90-
it("preserves both modifiers when chained", () => {
90+
it("preserves lowCardinality modifier and omits nullable when combined (nullable is in the type string)", () => {
9191
const type = t.string().lowCardinality().nullable();
9292
expect(type._modifiers.lowCardinality).toBe(true);
93-
expect(type._modifiers.nullable).toBe(true);
93+
expect(type._modifiers.nullable).toBeUndefined();
94+
expect(type._tinybirdType).toBe("LowCardinality(Nullable(String))");
95+
});
96+
97+
it("omits nullable modifier when nullable().lowCardinality() is chained", () => {
98+
const type = t.string().nullable().lowCardinality();
99+
expect(type._modifiers.lowCardinality).toBe(true);
100+
expect(type._modifiers.nullable).toBeUndefined();
101+
expect(type._tinybirdType).toBe("LowCardinality(Nullable(String))");
94102
});
95103
});
96104

src/schema/types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ function createValidator<TType, TTinybirdType extends string>(
106106
`LowCardinality(Nullable(${string}))`
107107
>(newType as `LowCardinality(Nullable(${string}))`, {
108108
...modifiers,
109-
nullable: true,
110109
}) as unknown as TypeValidator<
111110
TType | null,
112111
`Nullable(${TTinybirdType})`,
@@ -129,9 +128,10 @@ function createValidator<TType, TTinybirdType extends string>(
129128
// Extract base type from Nullable(X) and wrap as LowCardinality(Nullable(X))
130129
const baseType = tinybirdType.replace(/^Nullable\((.+)\)$/, "$1");
131130
const newType = `LowCardinality(Nullable(${baseType}))`;
131+
const { nullable: _, ...rest } = modifiers;
132132
return createValidator<TType, `LowCardinality(Nullable(${string}))`>(
133133
newType as `LowCardinality(Nullable(${string}))`,
134-
{ ...modifiers, lowCardinality: true },
134+
{ ...rest, lowCardinality: true },
135135
) as unknown as TypeValidator<
136136
TType,
137137
`LowCardinality(${TTinybirdType})`,

0 commit comments

Comments
 (0)