Skip to content

Commit f2a7780

Browse files
authored
quote table and col names from postgres and duckdb too (#6942)
## 📝 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). --> ## 🔍 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 ee80bd1 commit f2a7780

File tree

2 files changed

+37
-16
lines changed

2 files changed

+37
-16
lines changed

frontend/src/components/datasources/__tests__/utils.test.ts

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe("sqlCode", () => {
4848
sqlTableContext,
4949
});
5050
expect(result).toBe(
51-
'_df = mo.sql(f"""\nSELECT email FROM users LIMIT 100\n""")',
51+
'_df = mo.sql(f"""\nSELECT "email" FROM "users" LIMIT 100\n""")',
5252
);
5353
});
5454

@@ -68,7 +68,7 @@ describe("sqlCode", () => {
6868
sqlTableContext,
6969
});
7070
expect(result).toBe(
71-
'_df = mo.sql(f"""\nSELECT email FROM analytics.users LIMIT 100\n""")',
71+
'_df = mo.sql(f"""\nSELECT "email" FROM "analytics"."users" LIMIT 100\n""")',
7272
);
7373
});
7474

@@ -108,7 +108,7 @@ describe("sqlCode", () => {
108108
sqlTableContext,
109109
});
110110
expect(result).toBe(
111-
'_df = mo.sql(f"""\nSELECT email FROM remote.users LIMIT 100\n""")',
111+
'_df = mo.sql(f"""\nSELECT "email" FROM "remote"."users" LIMIT 100\n""")',
112112
);
113113
});
114114

@@ -127,7 +127,7 @@ describe("sqlCode", () => {
127127
sqlTableContext,
128128
});
129129
expect(result).toBe(
130-
'_df = mo.sql(f"""\nSELECT email FROM users LIMIT 100\n""")',
130+
'_df = mo.sql(f"""\nSELECT "email" FROM "users" LIMIT 100\n""")',
131131
);
132132

133133
const sqlTableContext2: SQLTableContext = {
@@ -144,7 +144,7 @@ describe("sqlCode", () => {
144144
sqlTableContext: sqlTableContext2,
145145
});
146146
expect(result2).toBe(
147-
'_df = mo.sql(f"""\nSELECT email FROM another_db.users LIMIT 100\n""")',
147+
'_df = mo.sql(f"""\nSELECT "email" FROM "another_db"."users" LIMIT 100\n""")',
148148
);
149149
});
150150
});
@@ -253,7 +253,7 @@ describe("sqlCode", () => {
253253
});
254254
});
255255

256-
describe("TimescaleDB dialect", () => {
256+
describe("TimescaleDB and PostgreSQL dialect", () => {
257257
it("should wrap table name with double quotes", () => {
258258
const sqlTableContext: SQLTableContext = {
259259
engine: "timescaledb",
@@ -270,7 +270,7 @@ describe("sqlCode", () => {
270270
sqlTableContext,
271271
});
272272
expect(result).toBe(
273-
'_df = mo.sql(f"""\nSELECT email FROM "users" LIMIT 100\n""", engine=timescaledb)',
273+
'_df = mo.sql(f"""\nSELECT "email" FROM "users" LIMIT 100\n""", engine=timescaledb)',
274274
);
275275
});
276276

@@ -290,17 +290,17 @@ describe("sqlCode", () => {
290290
sqlTableContext,
291291
});
292292
expect(result).toBe(
293-
'_df = mo.sql(f"""\nSELECT email FROM "remote"."sales"."users" LIMIT 100\n""", engine=timescaledb)',
293+
'_df = mo.sql(f"""\nSELECT "email" FROM "remote"."sales"."users" LIMIT 100\n""", engine=timescaledb)',
294294
);
295295
});
296296

297297
it("should handle schemaless database with double quotes", () => {
298298
const sqlTableContext: SQLTableContext = {
299-
engine: "timescaledb",
299+
engine: "postgres",
300300
schema: "",
301301
defaultDatabase: "mydb",
302302
database: "remote",
303-
dialect: "timescaledb",
303+
dialect: "postgres",
304304
};
305305

306306
const result = sqlCode({
@@ -309,7 +309,27 @@ describe("sqlCode", () => {
309309
sqlTableContext,
310310
});
311311
expect(result).toBe(
312-
'_df = mo.sql(f"""\nSELECT email FROM "remote"."users" LIMIT 100\n""", engine=timescaledb)',
312+
'_df = mo.sql(f"""\nSELECT "email" FROM "remote"."users" LIMIT 100\n""", engine=postgres)',
313+
);
314+
});
315+
316+
it("should not quote * column name", () => {
317+
const sqlTableContext: SQLTableContext = {
318+
engine: "postgres",
319+
schema: "public",
320+
defaultSchema: "public",
321+
defaultDatabase: "mydb",
322+
database: "mydb",
323+
dialect: "postgres",
324+
};
325+
326+
const result = sqlCode({
327+
table: mockTable,
328+
columnName: "*",
329+
sqlTableContext,
330+
});
331+
expect(result).toBe(
332+
'_df = mo.sql(f"""\nSELECT * FROM "users" LIMIT 100\n""", engine=postgres)',
313333
);
314334
});
315335
});

frontend/src/components/datasources/utils.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,21 @@ function getFormatter(dialect: string): SqlCodeFormatter {
5353
`SELECT TOP 100 ${columnName} FROM ${tableName}`,
5454
};
5555
case "timescaledb":
56+
case "postgres":
57+
case "postgresql":
58+
case "duckdb":
59+
// Quote column and table names to avoid raising errors on weird characters
5660
return {
57-
// TimescaleDB uses double quotes for identifiers
5861
formatTableName: (tableName: string) => {
5962
const parts = tableName.split(".");
6063
return parts.map((part) => `"${part}"`).join(".");
6164
},
62-
formatSelectClause: defaultFormatter.formatSelectClause,
65+
formatSelectClause: (columnName: string, tableName: string) =>
66+
`SELECT ${columnName === "*" ? "*" : `"${columnName}"`} FROM ${tableName} LIMIT 100`,
6367
};
64-
case "postgresql":
65-
case "postgres":
6668
case "db2":
6769
case "mysql":
6870
case "sqlite":
69-
case "duckdb":
7071
case "mariadb":
7172
case "cassandra":
7273
case "noql":

0 commit comments

Comments
 (0)