Skip to content

Commit 075a34c

Browse files
authored
sql mode - validate for duckdb (#6460)
## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> This runs queries in EXPLAIN mode on keypress. It's only enabled when feat flag is on & using Duckdb engine. The feat flag is not exposed. https://github.com/user-attachments/assets/e4d36acc-ab54-4136-ae3b-75934eeb0a94 ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> ## 📋 Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [x] I have added tests for the changes made. - [x] I have run the code and verified that it works as expected.
1 parent 50de3fd commit 075a34c

File tree

37 files changed

+998
-45
lines changed

37 files changed

+998
-45
lines changed

frontend/src/__mocks__/requests.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const MockRequestClient = {
4040
previewSQLTable: vi.fn().mockResolvedValue({}),
4141
previewSQLTableList: vi.fn().mockResolvedValue({ tables: [] }),
4242
previewDataSourceConnection: vi.fn().mockResolvedValue({}),
43+
validateSQL: vi.fn().mockResolvedValue({}),
4344
openFile: vi.fn().mockResolvedValue({}),
4445
getUsageStats: vi.fn().mockResolvedValue({}),
4546
sendPdb: vi.fn().mockResolvedValue({}),

frontend/src/components/editor/Cell.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ import { useDeleteCellCallback } from "./cell/useDeleteCell";
8080
import { useRunCell } from "./cell/useRunCells";
8181
import { HideCodeButton } from "./code/readonly-python-code";
8282
import { cellDomProps } from "./common";
83+
import { SqlValidationErrorBanner } from "./errors/sql-validation-errors";
8384
import { useCellNavigationProps } from "./navigation/navigation";
8485
import {
8586
useTemporarilyShownCode,
@@ -653,6 +654,7 @@ const EditableCellComponent = ({
653654
)}
654655
</div>
655656
</div>
657+
<SqlValidationErrorBanner cellId={cellId} />
656658
{cellOutput === "below" && outputArea}
657659
{cellRuntime.serialization && (
658660
<div className="py-1 px-2 flex items-center justify-end gap-2 last:rounded-b">
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
3+
import { AlertCircleIcon } from "lucide-react";
4+
import type { CellId } from "@/core/cells/ids";
5+
import { useSqlValidationErrorsForCell } from "@/core/codemirror/language/languages/sql/validation-errors";
6+
7+
export const SqlValidationErrorBanner = ({ cellId }: { cellId: CellId }) => {
8+
const error = useSqlValidationErrorsForCell(cellId);
9+
10+
if (!error) {
11+
return;
12+
}
13+
14+
return (
15+
<div className="p-3 text-sm flex flex-col text-muted-foreground gap-1.5 bg-destructive/5">
16+
<div className="flex items-start gap-1.5">
17+
<AlertCircleIcon size={13} className="mt-[3px] text-destructive" />
18+
<p>
19+
<span className="font-bold text-destructive">{error.errorType}:</span>{" "}
20+
{error.errorMessage}
21+
</p>
22+
</div>
23+
24+
{error.codeblock && (
25+
<pre
26+
lang="sql"
27+
className="text-xs bg-muted rounded p-2 pb-0 mx-3 overflow-x-auto font-mono whitespace-pre-wrap"
28+
>
29+
{error.codeblock}
30+
</pre>
31+
)}
32+
</div>
33+
);
34+
};

frontend/src/core/codemirror/language/__tests__/extension.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
languageAdapterState,
1515
switchLanguage,
1616
} from "../extension";
17+
import { exportedForTesting as sqlValidationErrorsForTesting } from "../languages/sql/validation-errors";
1718
import { languageMetadataField } from "../metadata";
1819

1920
let view: EditorView | null = null;
@@ -258,3 +259,26 @@ describe("switchLanguage", () => {
258259
});
259260
});
260261
});
262+
263+
describe("sqlValidationErrors", () => {
264+
const { splitErrorMessage } = sqlValidationErrorsForTesting;
265+
266+
describe("split error message", () => {
267+
it("should split the error message into error type and error message", () => {
268+
const error = "SyntaxError: SELECT * FROM df";
269+
const { errorType, errorMessage } = splitErrorMessage(error);
270+
expect(errorType).toBe("SyntaxError");
271+
expect(errorMessage).toBe("SELECT * FROM df");
272+
});
273+
274+
it("should handle multiple colons", () => {
275+
const error =
276+
"SyntaxError: SELECT * FROM df:SyntaxError: SELECT * FROM df";
277+
const { errorType, errorMessage } = splitErrorMessage(error);
278+
expect(errorType).toBe("SyntaxError");
279+
expect(errorMessage).toBe(
280+
"SELECT * FROM df:SyntaxError: SELECT * FROM df",
281+
);
282+
});
283+
});
284+
});
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
3+
import { describe, expect, it } from "vitest";
4+
import { exportedForTesting } from "../languages/sql/validation-errors";
5+
6+
describe("Error Message Splitting", () => {
7+
it("should handle error message splitting correctly", () => {
8+
const { splitErrorMessage } = exportedForTesting;
9+
10+
const result1 = splitErrorMessage("Syntax error: unexpected token");
11+
expect(result1.errorType).toBe("Syntax error");
12+
expect(result1.errorMessage).toBe("unexpected token");
13+
14+
const result2 = splitErrorMessage("Multiple: colons: in error");
15+
expect(result2.errorType).toBe("Multiple");
16+
expect(result2.errorMessage).toBe("colons: in error");
17+
18+
const result3 = splitErrorMessage("No colon error");
19+
expect(result3.errorType).toBe("No colon error");
20+
expect(result3.errorMessage).toBe("");
21+
});
22+
});
23+
24+
describe("DuckDB Error Handling", () => {
25+
it("should extract codeblock from error with LINE information", () => {
26+
const { handleDuckdbError } = exportedForTesting;
27+
28+
const error =
29+
'Binder Error: Referenced column "attacks" not found in FROM clause! Candidate bindings: "Attack", "Total" LINE 1:... from pokemon WHERE \'type_2\' = 32 and attack = 32 and not attacks = \'hi\' ^';
30+
31+
const result = handleDuckdbError(error);
32+
33+
expect(result.errorType).toBe("Binder Error");
34+
expect(result.errorMessage).toBe(
35+
'Referenced column "attacks" not found in FROM clause! Candidate bindings: "Attack", "Total"',
36+
);
37+
expect(result.codeblock).toBe(
38+
"LINE 1:... from pokemon WHERE 'type_2' = 32 and attack = 32 and not attacks = 'hi' ^",
39+
);
40+
});
41+
42+
it("should handle error without LINE information", () => {
43+
const { handleDuckdbError } = exportedForTesting;
44+
45+
const error = "Syntax Error: Invalid syntax near WHERE";
46+
47+
const result = handleDuckdbError(error);
48+
49+
expect(result.errorType).toBe("Syntax Error");
50+
expect(result.errorMessage).toBe("Invalid syntax near WHERE");
51+
expect(result.codeblock).toBeUndefined();
52+
});
53+
54+
it("should handle error with LINE at the beginning", () => {
55+
const { handleDuckdbError } = exportedForTesting;
56+
57+
const error = "LINE 1: SELECT * FROM table WHERE invalid_column = 1 ^";
58+
59+
const result = handleDuckdbError(error);
60+
61+
expect(result.errorType).toBe("LINE 1");
62+
expect(result.errorMessage).toBe(
63+
"SELECT * FROM table WHERE invalid_column = 1 ^",
64+
);
65+
expect(result.codeblock).toBeUndefined();
66+
});
67+
68+
it("should handle error with multiple LINE occurrences", () => {
69+
const { handleDuckdbError } = exportedForTesting;
70+
71+
const error =
72+
"Error: Something went wrong LINE 1: SELECT * FROM table WHERE invalid_column = 1 ^";
73+
74+
const result = handleDuckdbError(error);
75+
76+
expect(result.errorType).toBe("Error");
77+
expect(result.errorMessage).toBe("Something went wrong");
78+
expect(result.codeblock).toBe(
79+
"LINE 1: SELECT * FROM table WHERE invalid_column = 1 ^",
80+
);
81+
});
82+
83+
it("should handle complex error with nested quotes", () => {
84+
const { handleDuckdbError } = exportedForTesting;
85+
86+
const error =
87+
"Binder Error: Column \"name\" not found! LINE 1: SELECT * FROM users WHERE name = 'John' AND age > 25 ^";
88+
89+
const result = handleDuckdbError(error);
90+
91+
expect(result.errorType).toBe("Binder Error");
92+
expect(result.errorMessage).toBe('Column "name" not found!');
93+
expect(result.codeblock).toBe(
94+
"LINE 1: SELECT * FROM users WHERE name = 'John' AND age > 25 ^",
95+
);
96+
});
97+
98+
it("should handle error with LINE but no caret", () => {
99+
const { handleDuckdbError } = exportedForTesting;
100+
101+
const error = "Error: Invalid query LINE 1: SELECT * FROM table";
102+
103+
const result = handleDuckdbError(error);
104+
105+
expect(result.errorType).toBe("Error");
106+
expect(result.errorMessage).toBe("Invalid query");
107+
expect(result.codeblock).toBe("LINE 1: SELECT * FROM table");
108+
});
109+
110+
it("should trim whitespace from codeblock", () => {
111+
const { handleDuckdbError } = exportedForTesting;
112+
113+
const error = "Error: Something wrong LINE 1: SELECT * FROM table ^ ";
114+
115+
const result = handleDuckdbError(error);
116+
117+
expect(result.errorType).toBe("Error");
118+
expect(result.errorMessage).toBe("Something wrong");
119+
expect(result.codeblock).toBe("LINE 1: SELECT * FROM table ^");
120+
});
121+
122+
it("should handle empty error message", () => {
123+
const { handleDuckdbError } = exportedForTesting;
124+
125+
const error = "";
126+
127+
const result = handleDuckdbError(error);
128+
129+
expect(result.errorType).toBe("");
130+
expect(result.errorMessage).toBe("");
131+
expect(result.codeblock).toBeUndefined();
132+
});
133+
});
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
3+
import { useAtom } from "jotai";
4+
import { atomWithStorage } from "jotai/utils";
5+
import { store } from "@/core/state/jotai";
6+
7+
const BASE_KEY = "marimo:notebook-sql-mode";
8+
9+
export type SQLMode = "validate" | "default";
10+
11+
const sqlModeAtom = atomWithStorage<SQLMode>(BASE_KEY, "default");
12+
13+
export function useSQLMode() {
14+
const [sqlMode, setSQLMode] = useAtom(sqlModeAtom);
15+
return { sqlMode, setSQLMode };
16+
}
17+
18+
export function getSQLMode() {
19+
return store.get(sqlModeAtom);
20+
}

0 commit comments

Comments
 (0)