Skip to content

Commit 5c4ff8b

Browse files
mscolnickmscolnick
authored
fix: escape special characters in frontend altair chart builder (#6973)
Fixes #5956 This escapes special characters in the chart builder for both the generated python code and generated vega spec.
1 parent 17cfc88 commit 5c4ff8b

File tree

8 files changed

+434
-8
lines changed

8 files changed

+434
-8
lines changed

‎frontend/src/components/data-table/charts/__tests__/altair-generator.test.ts‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,4 +422,166 @@ describe("generateAltairChart", () => {
422422
)"
423423
`);
424424
});
425+
426+
it("should escape dots in field names", () => {
427+
const spec = createSpec({
428+
mark: "bar",
429+
encoding: {
430+
x: { field: "name.field" },
431+
y: { field: "value.data" },
432+
},
433+
});
434+
const datasource = "df";
435+
436+
const result = generateAltairChart(spec, datasource).toCode();
437+
438+
expect(result).toMatchInlineSnapshot(`
439+
"alt.Chart(df)
440+
.mark_bar()
441+
.encode(
442+
x=alt.X(field='name\\.field'),
443+
y=alt.Y(field='value\\.data')
444+
)"
445+
`);
446+
});
447+
448+
it("should escape brackets in field names", () => {
449+
const spec = createSpec({
450+
mark: "bar",
451+
encoding: {
452+
x: { field: "data[0]" },
453+
y: { field: "values[key]" },
454+
},
455+
});
456+
const datasource = "df";
457+
458+
const result = generateAltairChart(spec, datasource).toCode();
459+
460+
expect(result).toMatchInlineSnapshot(`
461+
"alt.Chart(df)
462+
.mark_bar()
463+
.encode(
464+
x=alt.X(field='data\\[0\\]'),
465+
y=alt.Y(field='values\\[key\\]')
466+
)"
467+
`);
468+
});
469+
470+
it("should escape colons in field names", () => {
471+
const spec = createSpec({
472+
mark: "bar",
473+
encoding: {
474+
x: { field: "time:stamp" },
475+
y: { field: "value" },
476+
},
477+
});
478+
const datasource = "df";
479+
480+
const result = generateAltairChart(spec, datasource).toCode();
481+
482+
expect(result).toMatchInlineSnapshot(`
483+
"alt.Chart(df)
484+
.mark_bar()
485+
.encode(
486+
x=alt.X(field='time\\:stamp'),
487+
y=alt.Y(field='value')
488+
)"
489+
`);
490+
});
491+
492+
it("should escape multiple special characters in field names", () => {
493+
const spec = createSpec({
494+
mark: "bar",
495+
encoding: {
496+
x: { field: "data[0].name:value" },
497+
y: { field: "result" },
498+
},
499+
});
500+
const datasource = "df";
501+
502+
const result = generateAltairChart(spec, datasource).toCode();
503+
504+
expect(result).toMatchInlineSnapshot(`
505+
"alt.Chart(df)
506+
.mark_bar()
507+
.encode(
508+
x=alt.X(field='data\\[0\\]\\.name\\:value'),
509+
y=alt.Y(field='result')
510+
)"
511+
`);
512+
});
513+
514+
it("should escape special characters in color encoding", () => {
515+
const spec = createSpec({
516+
mark: "bar",
517+
encoding: {
518+
x: { field: "category" },
519+
y: { field: "value" },
520+
color: { field: "group.name" },
521+
},
522+
});
523+
const datasource = "df";
524+
525+
const result = generateAltairChart(spec, datasource).toCode();
526+
527+
expect(result).toMatchInlineSnapshot(`
528+
"alt.Chart(df)
529+
.mark_bar()
530+
.encode(
531+
x=alt.X(field='category'),
532+
y=alt.Y(field='value'),
533+
color=alt.Color(field='group\\.name')
534+
)"
535+
`);
536+
});
537+
538+
it("should escape special characters in tooltip", () => {
539+
const spec = createSpec({
540+
mark: "bar",
541+
encoding: {
542+
x: { field: "category" },
543+
y: { field: "value" },
544+
tooltip: { field: "info.details" },
545+
},
546+
});
547+
const datasource = "df";
548+
549+
const result = generateAltairChart(spec, datasource).toCode();
550+
551+
expect(result).toMatchInlineSnapshot(`
552+
"alt.Chart(df)
553+
.mark_bar()
554+
.encode(
555+
x=alt.X(field='category'),
556+
y=alt.Y(field='value'),
557+
tooltip=alt.Tooltip(field='info\\.details')
558+
)"
559+
`);
560+
});
561+
562+
it("should escape special characters in facet encodings", () => {
563+
const spec = createSpec({
564+
mark: "bar",
565+
encoding: {
566+
x: { field: "category" },
567+
y: { field: "value" },
568+
row: { field: "row.label" },
569+
column: { field: "col[0]" },
570+
},
571+
});
572+
const datasource = "df";
573+
574+
const result = generateAltairChart(spec, datasource).toCode();
575+
576+
expect(result).toMatchInlineSnapshot(`
577+
"alt.Chart(df)
578+
.mark_bar()
579+
.encode(
580+
x=alt.X(field='category'),
581+
y=alt.Y(field='value'),
582+
row=alt.Row(field='row\\.label'),
583+
column=alt.Column(field='col\\[0\\]')
584+
)"
585+
`);
586+
});
425587
});

‎frontend/src/components/data-table/charts/__tests__/spec.test.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import type { PositionDef } from "vega-lite/build/src/channeldef";
44
import { describe, expect, it } from "vitest";
5+
import { invariant } from "@/utils/invariant";
56
import {
67
getAggregate,
78
getBinEncoding,
@@ -995,3 +996,136 @@ describe("getBinEncoding", () => {
995996
}
996997
});
997998
});
999+
1000+
describe("Special character escaping in specs", () => {
1001+
it("should escape dots in field names", () => {
1002+
const result = getAxisEncoding(
1003+
{
1004+
field: "value.data",
1005+
selectedDataType: "number",
1006+
aggregate: "sum",
1007+
timeUnit: undefined,
1008+
},
1009+
undefined,
1010+
"Value",
1011+
false,
1012+
ChartType.BAR,
1013+
);
1014+
1015+
invariant("field" in result, "field should be defined");
1016+
expect(result.field).toBe("value\\.data");
1017+
});
1018+
1019+
it("should escape brackets in field names", () => {
1020+
const result = getAxisEncoding(
1021+
{
1022+
field: "data[0]",
1023+
selectedDataType: "number",
1024+
aggregate: "mean",
1025+
timeUnit: undefined,
1026+
},
1027+
undefined,
1028+
"Data",
1029+
false,
1030+
ChartType.BAR,
1031+
);
1032+
1033+
invariant("field" in result, "field should be defined");
1034+
expect(result.field).toBe("data\\[0\\]");
1035+
});
1036+
1037+
it("should escape colons in field names", () => {
1038+
const result = getAxisEncoding(
1039+
{
1040+
field: "time:stamp",
1041+
selectedDataType: "temporal",
1042+
aggregate: NONE_VALUE,
1043+
timeUnit: "yearmonth",
1044+
},
1045+
undefined,
1046+
"Timestamp",
1047+
false,
1048+
ChartType.LINE,
1049+
);
1050+
1051+
invariant("field" in result, "field should be defined");
1052+
expect(result.field).toBe("time\\:stamp");
1053+
});
1054+
1055+
it("should escape multiple special characters in field names", () => {
1056+
const result = getAxisEncoding(
1057+
{
1058+
field: "data[0].value:result",
1059+
selectedDataType: "number",
1060+
aggregate: "sum",
1061+
timeUnit: undefined,
1062+
},
1063+
undefined,
1064+
"Result",
1065+
false,
1066+
ChartType.BAR,
1067+
);
1068+
1069+
invariant("field" in result, "field should be defined");
1070+
expect(result.field).toBe("data\\[0\\]\\.value\\:result");
1071+
});
1072+
1073+
it("should escape special characters in color encoding", () => {
1074+
const formValues: ChartSchemaType = {
1075+
general: {
1076+
colorByColumn: {
1077+
field: "group.name",
1078+
selectedDataType: "string",
1079+
aggregate: NONE_VALUE,
1080+
},
1081+
},
1082+
};
1083+
1084+
const result = getColorEncoding(ChartType.BAR, formValues);
1085+
1086+
invariant(result && "field" in result, "field should be defined");
1087+
expect(result?.field).toBe("group\\.name");
1088+
});
1089+
1090+
it("should escape special characters in tooltip field names", () => {
1091+
const formValues: ChartSchemaType = {
1092+
general: {
1093+
xColumn: {
1094+
field: "category",
1095+
selectedDataType: "string",
1096+
aggregate: NONE_VALUE,
1097+
},
1098+
yColumn: {
1099+
field: "value.data",
1100+
selectedDataType: "number",
1101+
aggregate: "sum",
1102+
},
1103+
},
1104+
tooltips: {
1105+
auto: true,
1106+
fields: [],
1107+
},
1108+
};
1109+
1110+
const xEncoding: PositionDef<string> = {
1111+
field: "category\\.escaped",
1112+
type: "nominal",
1113+
};
1114+
1115+
const yEncoding: PositionDef<string> = {
1116+
field: "value\\.data",
1117+
type: "quantitative",
1118+
aggregate: "sum",
1119+
};
1120+
1121+
const result = getTooltips({
1122+
formValues,
1123+
xEncoding,
1124+
yEncoding,
1125+
});
1126+
1127+
expect(result).toBeDefined();
1128+
expect(result?.[0]?.field).toBe("category\\.escaped");
1129+
expect(result?.[1]?.field).toBe("value\\.data");
1130+
});
1131+
});

‎frontend/src/components/data-table/charts/chart-spec/altair-generator.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
Variable,
99
VariableDeclaration,
1010
} from "@/utils/python-poet/poet";
11+
import { escapeFieldName } from "./utils";
1112

1213
/**
1314
* Generates Python code for an Altair chart.
@@ -154,7 +155,12 @@ function makeKwargs<T extends Record<string, any>>(obj: T) {
154155

155156
for (const [key, value] of Object.entries(obj)) {
156157
if (value !== undefined) {
157-
result[key] = new Literal(value);
158+
// Escape special characters in field names
159+
if (key === "field" && typeof value === "string") {
160+
result[key] = new Literal(escapeFieldName(value));
161+
} else {
162+
result[key] = new Literal(value);
163+
}
158164
}
159165
}
160166

‎frontend/src/components/data-table/charts/chart-spec/encodings.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from "../types";
2020
import { isFieldSet } from "./spec";
2121
import { convertDataTypeToVega } from "./types";
22+
import { escapeFieldName } from "./utils";
2223

2324
export function getBinEncoding(
2425
chartType: ChartType,
@@ -123,7 +124,7 @@ export function getColorEncoding(
123124
const aggregate = colorByColumn?.aggregate;
124125

125126
return {
126-
field: colorByColumn.field,
127+
field: escapeFieldName(colorByColumn.field),
127128
type: convertDataTypeToVega(selectedDataType),
128129
scale: getColorInScale(formValues),
129130
aggregate: getAggregate(aggregate, selectedDataType),
@@ -143,7 +144,7 @@ export function getOffsetEncoding(
143144
) {
144145
return undefined;
145146
}
146-
return { field: formValues.general?.colorByColumn?.field };
147+
return { field: escapeFieldName(formValues.general?.colorByColumn?.field) };
147148
}
148149

149150
export function getAggregate(

‎frontend/src/components/data-table/charts/chart-spec/spec.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
convertChartTypeToMark,
4242
convertDataTypeToVega,
4343
} from "./types";
44+
import { escapeFieldName } from "./utils";
4445

4546
/**
4647
* Convert marimo chart configuration to Vega-Lite specification.
@@ -178,7 +179,7 @@ export function getAxisEncoding(
178179
}
179180

180181
return {
181-
field: column.field,
182+
field: escapeFieldName(column.field),
182183
type: convertDataTypeToVega(column.selectedDataType || "unknown"),
183184
bin: getBinEncoding(chartType, selectedDataType, binValues),
184185
title: label,
@@ -211,7 +212,7 @@ export function getFacetEncoding(
211212
);
212213

213214
return {
214-
field: facet.field,
215+
field: escapeFieldName(facet.field),
215216
sort: facet.sort,
216217
timeUnit: getFacetTimeUnit(facet),
217218
type: convertDataTypeToVega(facet.selectedDataType || "unknown"),
@@ -244,7 +245,7 @@ function getPieChartSpec(
244245
);
245246

246247
const colorEncoding: ColorDef<string> = {
247-
field: colorByColumn.field,
248+
field: escapeFieldName(colorByColumn.field),
248249
type: convertDataTypeToVega(colorByColumn.selectedDataType || "unknown"),
249250
scale: getColorInScale(formValues),
250251
title: getFieldLabel(formValues.yAxis?.label),

0 commit comments

Comments
 (0)