github/dbx
4
0
Fork 0
mirror of https://github.com/t8y2/dbx.git synced 2026-10-20 00:17:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(query): prevent hidden ROWID in Oracle grouped queries

Browse Source
This commit is contained in:
lxk955
2026-10-11 23:53:52 +08:00
committed by GitHub
parent f73114a381
commit da281de1d1
8 changed files with 723 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/desktop/src/lib/__tests__/sql/editableQueryHiddenKeys.spec.ts
Vendored
+65
View File
@@ -177,6 +177,71 @@ describe("editable query hidden primary keys", () => {
).toBe("SELECT name, `id` AS `__DBX_PK_0` -- visible user name\nFROM users");
});
it("rejects queries with GROUP BY, HAVING, or set operations", () => {
expect(
buildQueryWithHiddenPrimaryKeys({
sql: "SELECT department, COUNT(*) AS total FROM users GROUP BY department",
databaseType: "oracle",
primaryKeys: ["__DBX_ROWID"],
existingResultNames: ["department", "total"],
sourceExpressions: { __DBX_ROWID: "ROWIDTOCHAR(ROWID)" },
}),
).toBeUndefined();
expect(
buildQueryWithHiddenPrimaryKeys({
sql: "SELECT department FROM users HAVING COUNT(*) > 1",
databaseType: "oracle",
primaryKeys: ["__DBX_ROWID"],
existingResultNames: ["department"],
sourceExpressions: { __DBX_ROWID: "ROWIDTOCHAR(ROWID)" },
}),
).toBeUndefined();
expect(
buildQueryWithHiddenPrimaryKeys({
sql: `SELECT
R.P_ID,
R.SUBJECT,
COUNT(0) AS CNT_NUM
FROM COMMON.RECORD R
WHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY
AND R.SUBJECT = '邮件提醒'
AND R.P_ID = '123214512'
GROUP BY R.P_ID, R.SUBJECT`,
databaseType: "oracle",
primaryKeys: ["__DBX_ROWID"],
existingResultNames: ["P_ID", "SUBJECT", "CNT_NUM"],
sourceExpressions: { __DBX_ROWID: "ROWIDTOCHAR(ROWID)" },
}),
).toBeUndefined();
expect(
buildQueryWithHiddenPrimaryKeys({
sql: "SELECT id, name FROM users UNION SELECT id, name FROM archived_users",
databaseType: "mysql",
primaryKeys: ["id"],
existingResultNames: ["id", "name"],
}),
).toBeUndefined();
});
it.each([
"SELECT name FROM users /* GROUP BY name HAVING COUNT(*) > 1 */",
"SELECT name FROM users -- GROUP BY name",
"SELECT name, 'GROUP BY name HAVING COUNT(*)' AS note FROM users",
'SELECT "GROUP", "HAVING", name FROM users',
"SELECT name, (SELECT COUNT(*) FROM orders GROUP BY user_id) AS total FROM users",
])("appends hidden keys when grouping text is outside the outer query: %s", (sql) => {
const result = buildQueryWithHiddenPrimaryKeys({ sql, databaseType: "oracle", primaryKeys: ["id"], existingResultNames: ["name", "note", "total"] });
expect(result?.sql).toContain('"id" AS "__DBX_PK_0"');
expect(result?.projections).toEqual([{ sourceName: "id", alias: "__DBX_PK_0" }]);
});
it.each(["UNION", "UNION ALL", "INTERSECT", "EXCEPT", "MINUS"])("does not append hidden keys across %s", (operation) => {
expect(buildQueryWithHiddenPrimaryKeys({ sql: `SELECT name FROM users ${operation} SELECT name FROM archived_users`, databaseType: "oracle", primaryKeys: ["id"], existingResultNames: ["name"] })).toBeUndefined();
});
it("analyzes SQL Server TOP projections as ordinary source columns", () => {
for (const sql of [
"SELECT TOP 2 name, note FROM dbo.users ORDER BY name",
apps/desktop/src/lib/__tests__/sql/groupByColumnMapping.spec.ts
Vendored
+24
View File
@@ -58,6 +58,30 @@ describe("grouped query display column mapping", () => {
expect(analysis.hasHavingClause).toBe(true);
});
it("evaluates issue 11672 query editability", () => {
const sqls = [
`SELECT
R.P_ID,
R.SUBJECT,
COUNT(0) AS CNT_NUM
FROM COMMON.RECORD R
WHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY
AND R.SUBJECT = '邮件提醒'
AND R.P_ID = '123214512'
GROUP BY R.P_ID, R.SUBJECT`,
`SELECT\r\nR.P_ID,\r\nR.SUBJECT,\r\nCOUNT(0) AS CNT_NUM\r\nFROM COMMON.RECORD R\r\nWHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY\r\nAND R.SUBJECT = '邮件提醒'\r\nAND R.P_ID = '123214512'\r\nGROUP BY R.P_ID, R.SUBJECT`,
`SELECT R.P_ID, R.SUBJECT, COUNT(0) AS CNT_NUM FROM COMMON.RECORD R WHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY AND R.SUBJECT = '邮件提醒' AND R.P_ID = '123214512' GROUP BY R.P_ID, R.SUBJECT`,
`SELECT R.P_ID, R.SUBJECT, COUNT(0) AS CNT_NUM FROM COMMON.RECORD R GROUP BY R.P_ID, R.SUBJECT`,
];
for (const sql of sqls) {
const editability = analyzeEditableQueryEditability(sql);
expect(editability.editable).toBe(false);
if (!editability.editable) {
expect(editability.reason).toBe("aggregation");
}
}
});
it("records window functions so mutation safety can reject them", () => {
const analysis = analyzeSelectStructureForDisplay("SELECT id, ROW_NUMBER() OVER (ORDER BY id) AS row_number, COUNT(*) FROM users GROUP BY id");
expect(analysis).not.toBeNull();
apps/desktop/src/lib/sql/editableQueryHiddenKeys.ts
+2
View File
@@ -47,6 +47,8 @@ function appendSelectProjections(sql: string, expressions: string[], databaseTyp
const tokens = tokenizeSqlSemantic(sql, sqlSemanticDialectFor({ databaseType }).id);
const selectIndex = tokens.findIndex((token) => token.kind === "word" && token.depth === 0 && token.normalized === "select");
if (selectIndex < 0) return undefined;
const hasGroupingOrSet = tokens.some((token) => token.kind === "word" && token.depth === 0 && (token.normalized === "group" || token.normalized === "having" || token.normalized === "union" || token.normalized === "intersect" || token.normalized === "except" || token.normalized === "minus"));
if (hasGroupingOrSet) return undefined;
const fromIndex = tokens.findIndex((token, index) => index > selectIndex && token.kind === "word" && token.depth === 0 && token.normalized === "from");
if (fromIndex < 0) return undefined;
apps/desktop/src/lib/sql/sqlAnalysis.ts
+128 -10
View File
@@ -1,5 +1,7 @@
import type { DatabaseType } from "@/types/database";
import { DBX_ROWID_COLUMN } from "@/lib/table/tableEditing";
import { sqlSemanticDialectFor } from "@/lib/sql/semantic/dialect";
import { tokenizeSqlSemantic } from "@/lib/sql/semantic/tokens";
// Binary column types that should not be edited inline
export const BINARY_TYPES = new Set(["blob", "clob", "bytea", "varbinary", "binary", "image", "longblob", "mediumblob", "tinyblob", "blob sub_type 2004", "blob sub_type 2005"]);
@@ -266,6 +268,7 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability {
// so it cannot be mapped to base rows like an ordinary DISTINCT projection.
if (distinct && /^ON\b/i.test(selectBodyWithoutDistinct)) return { editable: false, reason: "aggregation" };
const selectBody = stripSqlServerTopClause(selectBodyWithoutDistinct);
if (containsTopLevelAggregate(selectBody)) return { editable: false, reason: "aggregation" };
const groupIndex = findTopLevelKeyword(normalized, "GROUP", fromIndex + 4);
const havingIndex = findTopLevelKeyword(normalized, "HAVING", fromIndex + 4);
@@ -275,7 +278,7 @@ export function analyzeEditableQueryEditability(sql: string): QueryEditability {
// Row-locking FOR clauses change concurrency behavior, not the base-row
// mapping. Output/read-only FOR modes must stay non-editable.
if (forIndex >= 0 && !/^FOR\s+(?:UPDATE|SHARE|NO\s+KEY\s+UPDATE|KEY\s+SHARE)\b/i.test(normalized.slice(forIndex))) return { editable: false, reason: "complex-source" };
const fromEnd = firstTopLevelKeywordIndex(normalized, ["WHERE", "ORDER", "LIMIT", "OFFSET", "FETCH", "FOR"], fromIndex + 4);
const fromEnd = firstTopLevelKeywordIndex(normalized, ["WHERE", "GROUP", "HAVING", "ORDER", "LIMIT", "OFFSET", "FETCH", "FOR"], fromIndex + 4);
const fromBody = normalized.slice(fromIndex + 4, fromEnd < 0 ? normalized.length : fromEnd).trim();
if (isExternalFromSource(fromBody)) return { editable: false, reason: "external-source" };
const sources = parseFromSources(fromBody);
@@ -799,21 +802,128 @@ function skipWhitespace(text: string, pos: number): number {
return pos;
}
function stripSqlComments(sql: string): string {
return sql.replace(/--.*$/gm, "").replace(/\/\*[\s\S]*?\*\//g, "");
const AGGREGATE_FUNCTION_PATTERN = /\b(?:COUNT|SUM|AVG|MIN|MAX|EVERY|ANY|SOME|STDDEV(?:_POP|_SAMP)?|VAR(?:IANCE|_POP|_SAMP)?|GROUP_CONCAT|STRING_AGG|LISTAGG|ARRAY_AGG|JSON_ARRAYAGG|JSON_OBJECTAGG|XMLAGG|APPROX_COUNT_DISTINCT)\s*\(/i;
function containsTopLevelAggregate(expression: string): boolean {
let quote: string | null = null;
let cleaned = "";
let depth = 0;
let subqueryDepth: number | undefined;
for (let i = 0; i < expression.length; i++) {
const ch = expression[i]!;
if (quote) {
if (ch === quote) {
if (quote === "'" && expression[i + 1] === "'") {
i++;
continue;
}
quote = null;
} else if (quote === "]" && ch === "]") {
quote = null;
}
continue;
}
if (ch === "'" || ch === '"' || ch === "`") {
quote = ch;
cleaned += " ";
continue;
}
if (ch === "[") {
quote = "]";
cleaned += " ";
continue;
}
if (ch === "(") depth++;
if (ch === ")") {
depth--;
if (subqueryDepth !== undefined && depth < subqueryDepth) subqueryDepth = undefined;
}
if (subqueryDepth !== undefined) continue;
if (depth > 0 && (ch === "S" || ch === "s") && expression.slice(i, i + 6).toUpperCase() === "SELECT" && !isIdentifierChar(expression[i - 1] ?? "") && !isIdentifierChar(expression[i + 6] ?? "")) {
subqueryDepth = depth;
cleaned += " ";
continue;
}
cleaned += ch;
}
return AGGREGATE_FUNCTION_PATTERN.test(cleaned);
}
function hasTopLevelKeyword(sql: string, keywords: string[]): boolean {
return keywords.some((keyword) => findTopLevelKeyword(sql, keyword, 0) >= 0);
function stripSqlComments(sql: string): string {
let result = "";
let quote: string | null = null;
let i = 0;
while (i < sql.length) {
const ch = sql[i]!;
if (quote) {
result += ch;
if (ch === quote) {
if (quote === "'" && sql[i + 1] === "'") {
result += sql[i + 1]!;
i += 2;
continue;
}
quote = null;
}
i++;
continue;
}
if (ch === "'" || ch === '"' || ch === "`") {
quote = ch;
result += ch;
i++;
continue;
}
if (ch === "[") {
quote = "]";
result += ch;
i++;
continue;
}
if (ch === "-" && sql[i + 1] === "-") {
i += 2;
while (i < sql.length && sql[i] !== "\n") {
i++;
}
continue;
}
if (ch === "/" && sql[i + 1] === "*") {
i += 2;
while (i < sql.length && !(sql[i] === "*" && sql[i + 1] === "/")) {
i++;
}
if (i < sql.length) i += 2;
continue;
}
result += ch;
i++;
}
return result;
}
export function hasTopLevelKeyword(sql: string, keywords: string[], databaseType?: DatabaseType): boolean {
const dialect = sqlSemanticDialectFor({ databaseType });
const mysqlFamily = dialect.id === "mysql" || dialect.id === "doris";
const tokens = tokenizeSqlSemantic(sql, dialect.id, { mysqlDashCommentRequiresWhitespace: mysqlFamily, mysqlBackslashEscape: mysqlFamily });
const normalizedKeywords = new Set(keywords.map((keyword) => keyword.toLowerCase()));
return tokens.some((token) => token.kind === "word" && token.depth === 0 && normalizedKeywords.has(token.normalized));
}
function hasTopLevelSemicolon(sql: string): boolean {
let depth = 0;
let quote: string | null = null;
for (let i = 0; i < sql.length; i++) {
const ch = sql[i];
const ch = sql[i]!;
if (quote) {
if (ch === quote || (quote === "]" && ch === "]")) quote = null;
if (ch === quote) {
if (quote === "'" && sql[i + 1] === "'") {
i++;
continue;
}
quote = null;
} else if (quote === "]" && ch === "]") {
quote = null;
}
continue;
}
if (ch === "'" || ch === '"' || ch === "`") {
@@ -847,9 +957,17 @@ function findTopLevelKeyword(sql: string, keyword: string, start: number): numbe
let quote: string | null = null;
const upperKeyword = keyword.toUpperCase();
for (let i = start; i < sql.length; i++) {
const ch = sql[i];
const ch = sql[i]!;
if (quote) {
if (ch === quote || (quote === "]" && ch === "]")) quote = null;
if (ch === quote) {
if (quote === "'" && sql[i + 1] === "'") {
i++;
continue;
}
quote = null;
} else if (quote === "]" && ch === "]") {
quote = null;
}
continue;
}
if (ch === "'" || ch === '"' || ch === "`") {
@@ -870,7 +988,7 @@ function findTopLevelKeyword(sql: string, keyword: string, start: number): numbe
}
if (depth !== 0) continue;
if (sql.slice(i, i + keyword.length).toUpperCase() !== upperKeyword) continue;
const before = i === 0 ? "" : sql[i - 1];
const before = i === 0 ? "" : sql[i - 1]!;
const after = sql[i + keyword.length] ?? "";
if (!isIdentifierChar(before) && !isIdentifierChar(after)) return i;
}
apps/desktop/src/stores/__tests__/queryStore.hiddenPrimaryKey.spec.ts
Vendored
+186
View File
@@ -544,6 +544,192 @@ describe("queryStore hidden primary key editing", () => {
expect(tab.queryEditabilityReason).toBeUndefined();
});
it.each([
{ databaseType: "mysql" as const, suffix: "/* GROUP BY name HAVING COUNT(*) > 1 */" },
{ databaseType: "mysql" as const, suffix: "-- GROUP BY name" },
{ databaseType: "mysql" as const, suffix: "WHERE name = 'GROUP BY name HAVING COUNT(*)'" },
{ databaseType: "mysql" as const, suffix: "WHERE id IN (SELECT user_id FROM orders GROUP BY user_id HAVING COUNT(*) > 1)" },
{ databaseType: "oracle" as const, suffix: "/* GROUP BY name HAVING COUNT(*) > 1 */" },
{ databaseType: "oracle" as const, suffix: "-- GROUP BY name" },
{ databaseType: "oracle" as const, suffix: "WHERE name = 'GROUP BY name HAVING COUNT(*)'" },
{ databaseType: "oracle" as const, suffix: "WHERE id IN (SELECT user_id FROM orders GROUP BY user_id HAVING COUNT(*) > 1)" },
])("keeps omitted-key editing for $databaseType with nonstructural grouping text: $suffix", async ({ databaseType, suffix }) => {
const connectionId = `${databaseType}-1`;
const database = databaseType === "oracle" ? "ORCL" : "app";
getConnectionConfig.mockReturnValue({ id: connectionId, name: databaseType, db_type: databaseType, database, query_timeout_secs: 30 });
const { analyzeEditableQueryEditability: analyzeSqlEditability } = await import("@/lib/sql/sqlAnalysis");
analyzeEditableQueryEditability.mockImplementation(async (sql: string) => analyzeSqlEditability(sql));
const hiddenKey = databaseType === "oracle" ? '"id" AS "__DBX_PK_0"' : "`id` AS `__DBX_PK_0`";
executeMulti.mockResolvedValue([{ columns: ["name", "__DBX_PK_0"], rows: [["Alice", 7]], affected_rows: 0, execution_time_ms: 1 }]);
const { useQueryStore } = await import("@/stores/queryStore");
const store = useQueryStore();
const tabId = store.createTab(connectionId, database, "Query");
store.setAutoCommit(tabId, true);
const sql = `SELECT name FROM users ${suffix}`;
for (let execution = 0; execution < 2; execution++) {
await store.executeTabSql(tabId, sql);
expect(executeMulti).toHaveBeenLastCalledWith(connectionId, database, `SELECT name, ${hiddenKey} FROM users ${suffix}`, undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 }));
const tab = store.tabs.find((item) => item.id === tabId)!;
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["name", "id"]));
expect(tab.queryAnalysis).toBeDefined();
expect(tab.queryEditabilityReason).toBeUndefined();
expect(tab.result?.hidden_column_indexes).toEqual([1]);
}
});
it("keeps hidden-key editing for quoted grouping identifiers", async () => {
getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 });
getColumns.mockResolvedValue([
{ name: "id", data_type: "NUMBER", is_nullable: false, is_primary_key: true },
{ name: "GROUP", data_type: "VARCHAR2", is_nullable: true, is_primary_key: false },
{ name: "HAVING", data_type: "VARCHAR2", is_nullable: true, is_primary_key: false },
{ name: "name", data_type: "VARCHAR2", is_nullable: true, is_primary_key: false },
]);
const { analyzeEditableQueryEditability: analyzeSqlEditability } = await import("@/lib/sql/sqlAnalysis");
analyzeEditableQueryEditability.mockImplementation(async (sql: string) => analyzeSqlEditability(sql));
executeMulti.mockResolvedValue([{ columns: ["GROUP", "HAVING", "name", "__DBX_PK_0"], rows: [["a", "b", "Alice", 7]], affected_rows: 0, execution_time_ms: 1 }]);
const { useQueryStore } = await import("@/stores/queryStore");
const store = useQueryStore();
const tabId = store.createTab("oracle-1", "ORCL", "Query");
store.setAutoCommit(tabId, true);
await store.executeTabSql(tabId, 'SELECT "GROUP", "HAVING", name FROM users');
expect(executeMulti.mock.calls.at(-1)?.[2]).toContain('"id" AS "__DBX_PK_0"');
const tab = store.tabs.find((item) => item.id === tabId)!;
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["GROUP", "HAVING", "name", "id"]));
expect(tab.queryAnalysis).toBeDefined();
expect(tab.queryEditabilityReason).toBeUndefined();
});
it("keeps keyless Oracle ROWID editing when comments mention grouping", async () => {
getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", default_schema: "APP", query_timeout_secs: 30 });
getColumns.mockResolvedValue([{ name: "NAME", data_type: "VARCHAR2", is_nullable: true, is_primary_key: false }]);
lookupLocalCompletionTables.mockReturnValue([{ name: "USERS", schema: "APP", type: "table" }]);
const { analyzeEditableQueryEditability: analyzeSqlEditability } = await import("@/lib/sql/sqlAnalysis");
analyzeEditableQueryEditability.mockImplementation(async (sql: string) => analyzeSqlEditability(sql));
executeMulti.mockResolvedValue([{ columns: ["NAME", "__DBX_PK_0"], rows: [["Alice", "rowid-1"]], affected_rows: 0, execution_time_ms: 1 }]);
const { useQueryStore } = await import("@/stores/queryStore");
const store = useQueryStore();
const tabId = store.createTab("oracle-1", "ORCL", "Query");
store.setAutoCommit(tabId, true);
await store.executeTabSql(tabId, "SELECT NAME FROM APP.USERS /* GROUP BY NAME HAVING COUNT(*) > 1 */");
expect(executeMulti.mock.calls.at(-1)?.[2]).toContain('ROWIDTOCHAR(ROWID) AS "__DBX_PK_0"');
const tab = store.tabs.find((item) => item.id === tabId)!;
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["NAME", "__DBX_ROWID"]));
expect(tab.tableMeta?.primaryKeys).toEqual(["__DBX_ROWID"]);
expect(tab.queryAnalysis).toBeDefined();
expect(tab.queryEditabilityReason).toBeUndefined();
});
it.each([
{ databaseType: "mysql" as const, returnedKey: false },
{ databaseType: "mysql" as const, returnedKey: true },
{ databaseType: "oracle" as const, returnedKey: false },
{ databaseType: "oracle" as const, returnedKey: true },
])("keeps scalar-subquery projections editable for $databaseType with returnedKey=$returnedKey", async ({ databaseType, returnedKey }) => {
const connectionId = `${databaseType}-1`;
const database = databaseType === "oracle" ? "ORCL" : "app";
getConnectionConfig.mockReturnValue({ id: connectionId, name: databaseType, db_type: databaseType, database, query_timeout_secs: 30 });
const { analyzeEditableQueryEditability: analyzeSqlEditability } = await import("@/lib/sql/sqlAnalysis");
analyzeEditableQueryEditability.mockImplementation(async (sql: string) => analyzeSqlEditability(sql));
const columns = returnedKey ? ["id", "name", "total"] : ["name", "total", "__DBX_PK_0"];
const rows = returnedKey ? [[7, "Alice", 2]] : [["Alice", 2, 7]];
executeMulti.mockResolvedValue([{ columns, rows, affected_rows: 0, execution_time_ms: 1 }]);
const { useQueryStore } = await import("@/stores/queryStore");
const store = useQueryStore();
const tabId = store.createTab(connectionId, database, "Query");
store.setAutoCommit(tabId, true);
const sql = `SELECT ${returnedKey ? "u.id, " : ""}u.name, (SELECT COUNT(*) FROM orders o WHERE o.user_id = u.id) AS total FROM users u`;
await store.executeTabSql(tabId, sql);
if (returnedKey) {
expect(executeMulti).toHaveBeenLastCalledWith(connectionId, database, sql, undefined, expect.any(String), expect.any(Object));
} else {
expect(executeMulti.mock.calls.at(-1)?.[2]).toContain("__DBX_PK_0");
}
const tab = store.tabs.find((item) => item.id === tabId)!;
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(returnedKey ? ["id", "name", undefined] : ["name", undefined, "id"]));
expect(tab.queryAnalysis).toBeDefined();
expect(tab.queryEditabilityReason).toBeUndefined();
expect(tab.queryAnalysis?.columns.find((column) => column.resultName === "total")?.sourceName).toBeUndefined();
});
it.each(["SELECT name FROM users GROUP BY name", "SELECT name FROM users HAVING COUNT(*) > 1", "SELECT COALESCE(SUM(id), 0) AS total FROM users", "SELECT name FROM users UNION SELECT name FROM archived_users"])("keeps structural grouping and set queries read-only: %s", async (sql) => {
getConnectionConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 });
const { analyzeEditableQueryEditability: analyzeSqlEditability } = await import("@/lib/sql/sqlAnalysis");
analyzeEditableQueryEditability.mockImplementation(async (query: string) => analyzeSqlEditability(query));
executeMulti.mockResolvedValue([{ columns: ["name"], rows: [["Alice"]], affected_rows: 0, execution_time_ms: 1 }]);
const { useQueryStore } = await import("@/stores/queryStore");
const store = useQueryStore();
const tabId = store.createTab("oracle-1", "ORCL", "Query");
store.setAutoCommit(tabId, true);
await store.executeTabSql(tabId, sql);
expect(executeMulti).toHaveBeenLastCalledWith("oracle-1", "ORCL", sql, undefined, expect.any(String), expect.any(Object));
const tab = store.tabs.find((item) => item.id === tabId)!;
await vi.waitFor(() => expect(tab.queryEditabilityReason).toBe(sql.includes("UNION") ? "set-operation" : "aggregation"));
expect(tab.queryAnalysis).toBeUndefined();
expect(tab.result?.hidden_column_indexes).toBeUndefined();
});
it("never rewrites an Oracle GROUP BY query with hidden ROWID", async () => {
getConnectionConfig.mockReturnValue({
id: "oracle-1",
name: "Oracle",
db_type: "oracle",
database: "ORCL",
query_timeout_secs: 30,
});
getColumns.mockResolvedValue([
{ name: "P_ID", type_name: "VARCHAR2", nullable: true },
{ name: "SUBJECT", type_name: "VARCHAR2", nullable: true },
{ name: "CREATE_TIME", type_name: "DATE", nullable: true },
]);
listTables.mockResolvedValue([{ name: "RECORD", table_type: "TABLE" }]);
listIndexes.mockResolvedValue([]);
analyzeEditableQueryEditability.mockResolvedValue({
editable: false,
reason: "aggregation",
});
executeMulti.mockResolvedValue([
{
columns: ["P_ID", "SUBJECT", "CNT_NUM"],
rows: [["123214512", "邮件提醒", 1]],
affected_rows: 0,
execution_time_ms: 1,
},
]);
const { useQueryStore } = await import("@/stores/queryStore");
const store = useQueryStore();
const tabId = store.createTab("oracle-1", "ORCL", "Query");
store.setAutoCommit(tabId, true);
const sql = `SELECT
R.P_ID,
R.SUBJECT,
COUNT(0) AS CNT_NUM
FROM COMMON.RECORD R
WHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY
AND R.SUBJECT = '邮件提醒'
AND R.P_ID = '123214512'
GROUP BY R.P_ID, R.SUBJECT`;
// Execute first time (populates metadata cache)
await store.executeTabSql(tabId, sql);
expect(executeMulti).toHaveBeenLastCalledWith("oracle-1", "ORCL", sql, undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 }));
// Execute second time (cache hit within 30s)
await store.executeTabSql(tabId, sql);
expect(executeMulti).toHaveBeenLastCalledWith("oracle-1", "ORCL", sql, undefined, expect.any(String), expect.objectContaining({ timeoutSecs: 30 }));
expect(executeMulti).not.toHaveBeenCalledWith("oracle-1", "ORCL", expect.stringContaining("ROWIDTOCHAR"), undefined, expect.any(String), expect.any(Object));
const tab = store.tabs.find((item) => item.id === tabId)!;
expect(tab.result?.hidden_column_indexes).toBeUndefined();
});
it.each([
{
connection: { id: "oracle-1", name: "Oracle", db_type: "oracle", database: "ORCL", query_timeout_secs: 30 },
apps/desktop/src/stores/queryStore.ts
+4
View File
@@ -41,6 +41,7 @@ import {
analyzeEditableQueryEditability,
analyzeSelectStructureForDisplay,
foldUnquotedPostgresMetadataIdentifier,
hasTopLevelKeyword,
resolveMetadataColumnName,
resolveSourceColumnsByOrdinal,
sourceColumnsForResult,
@@ -6411,6 +6412,7 @@ export const useQueryStore = defineStore("query", () => {
const stableLobSource = databaseType === "oracle" ? oracleRowIdIsSafeForQuery(tab, loaded) : databaseType === "db2";
const largeValuePreview = (databaseType === "oracle" || databaseType === "db2") && primaryKeys.length > 0 && stableLobSource && columnsAllowDeferredLobMarkers(loaded.tableMeta.columns) && queryProjectsDeferredLob(databaseType, metadataAnalysis, loaded.source.key, loaded.tableMeta.columns);
const unchanged = { sql, metadataSql: sql, hiddenPrimaryKeys: [], largeValuePreview };
if (hasTopLevelKeyword(sql, ["GROUP", "HAVING"], databaseType)) return unchanged;
const missingPrimaryKeys =
declaredPrimaryKeys.length === 0
? primaryKeys.filter(
@@ -6444,9 +6446,11 @@ export const useQueryStore = defineStore("query", () => {
if (!databaseType || !HIDDEN_QUERY_KEY_DATABASE_TYPES.has(databaseType) || !tab.connectionId) return unchanged;
try {
if (hasTopLevelKeyword(sql, ["GROUP", "HAVING"], databaseType)) return unchanged;
const editability = analyzeEditableQueryEditability(sql);
if (!editability.editable || !editability.analysis) return unchanged;
const analysis = editability.analysis;
if (analysis.groupByColumns?.length || analysis.hasHavingClause) return unchanged;
const sources = editableQuerySources(analysis);
if (sources.length !== 1 || analysis.distinct) return unchanged;
const source = sources[0]!;
crates/dbx-sql-core/src/sql_editability.rs
+215 -19
View File
@@ -163,6 +163,9 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability {
(raw_select_body, false)
};
let select_body = strip_sql_server_top_clause(select_body);
if contains_top_level_aggregate(select_body) {
return not_editable(QueryEditabilityReason::Aggregation);
}
let group_index = find_top_level_keyword(&normalized, "GROUP", from_index + "FROM".len());
let having_index = find_top_level_keyword(&normalized, "HAVING", from_index + "FROM".len());
@@ -179,7 +182,7 @@ pub fn analyze_editable_query_editability(sql: &str) -> QueryEditability {
}
let from_end = first_top_level_keyword_index(
&normalized,
&["WHERE", "ORDER", "LIMIT", "OFFSET", "FETCH", "FOR"],
&["WHERE", "GROUP", "HAVING", "ORDER", "LIMIT", "OFFSET", "FETCH", "FOR"],
from_body_start,
)
.unwrap_or(normalized.len());
@@ -958,32 +961,141 @@ fn skip_whitespace(text: &str, pos: usize) -> usize {
current
}
fn contains_top_level_aggregate(expression: &str) -> bool {
let mut quote: Option<char> = None;
let mut cleaned = String::new();
let mut depth = 0i32;
let mut subquery_depth: Option<i32> = None;
let mut chars = expression.char_indices().peekable();
while let Some((index, ch)) = chars.next() {
if let Some(close) = quote {
if ch == close {
if close == '\'' && chars.peek().is_some_and(|(_, next)| *next == '\'') {
chars.next();
continue;
}
quote = None;
}
continue;
}
match ch {
'\'' | '"' | '`' | '[' => {
quote = Some(if ch == '[' { ']' } else { ch });
cleaned.push(' ');
continue;
}
'(' => depth += 1,
')' => {
depth -= 1;
if subquery_depth.is_some_and(|query_depth| depth < query_depth) {
subquery_depth = None;
}
}
_ => {}
}
if subquery_depth.is_some() {
continue;
}
if depth > 0
&& (ch == 'S' || ch == 's')
&& starts_with_keyword(&expression[index..], "SELECT")
&& !previous_char(expression, index).is_some_and(is_identifier_char)
{
subquery_depth = Some(depth);
cleaned.push(' ');
continue;
}
cleaned.push(ch);
}
const AGGREGATE_FUNCTIONS: &[&str] = &[
"COUNT",
"SUM",
"AVG",
"MIN",
"MAX",
"EVERY",
"ANY",
"SOME",
"STDDEV",
"STDDEV_POP",
"STDDEV_SAMP",
"VARIANCE",
"VAR_POP",
"VAR_SAMP",
"GROUP_CONCAT",
"STRING_AGG",
"LISTAGG",
"ARRAY_AGG",
"JSON_ARRAYAGG",
"JSON_OBJECTAGG",
"XMLAGG",
"APPROX_COUNT_DISTINCT",
];
let upper = cleaned.to_ascii_uppercase();
for agg in AGGREGATE_FUNCTIONS {
let mut search_from = 0;
while let Some(pos) = upper[search_from..].find(agg) {
let idx = search_from + pos;
search_from = idx + agg.len();
let before = previous_char(&upper, idx);
if before.is_some_and(is_identifier_char) {
continue;
}
let tail = skip_whitespace(&upper, idx + agg.len());
if upper[tail..].starts_with('(') {
return true;
}
}
}
false
}
fn strip_sql_comments(sql: &str) -> String {
let mut result = String::new();
let mut quote: Option<char> = None;
let mut chars = sql.chars().peekable();
while let Some(ch) = chars.next() {
if ch == '-' && chars.peek() == Some(&'-') {
chars.next();
for next in chars.by_ref() {
if next == '\n' {
result.push('\n');
break;
if let Some(close) = quote {
result.push(ch);
if ch == close {
if close == '\'' && chars.peek() == Some(&'\'') {
result.push(chars.next().unwrap());
continue;
}
quote = None;
}
continue;
}
if ch == '/' && chars.peek() == Some(&'*') {
chars.next();
let mut previous = '\0';
for next in chars.by_ref() {
if previous == '*' && next == '/' {
break;
}
previous = next;
match ch {
'\'' | '"' | '`' => {
quote = Some(ch);
result.push(ch);
}
continue;
'[' => {
quote = Some(']');
result.push(ch);
}
'-' if chars.peek() == Some(&'-') => {
chars.next();
for next in chars.by_ref() {
if next == '\n' {
result.push('\n');
break;
}
}
}
'/' if chars.peek() == Some(&'*') => {
chars.next();
let mut previous = '\0';
for next in chars.by_ref() {
if previous == '*' && next == '/' {
break;
}
previous = next;
}
}
_ => result.push(ch),
}
result.push(ch);
}
result
}
@@ -995,9 +1107,14 @@ fn has_top_level_keyword(sql: &str, keywords: &[&str]) -> bool {
fn has_top_level_semicolon(sql: &str) -> bool {
let mut depth = 0i32;
let mut quote: Option<char> = None;
for ch in sql.chars() {
let mut chars = sql.chars().peekable();
while let Some(ch) = chars.next() {
if let Some(close) = quote {
if ch == close {
if close == '\'' && chars.peek() == Some(&'\'') {
chars.next();
continue;
}
quote = None;
}
continue;
@@ -1023,9 +1140,14 @@ fn find_top_level_keyword(sql: &str, keyword: &str, start: usize) -> Option<usiz
let mut quote: Option<char> = None;
let upper_keyword = keyword.to_ascii_uppercase();
for (index, ch) in sql.char_indices().filter(|(index, _)| *index >= start) {
let mut iter = sql.char_indices().filter(|(index, _)| *index >= start).peekable();
while let Some((index, ch)) = iter.next() {
if let Some(close) = quote {
if ch == close {
if close == '\'' && iter.peek().map(|(_, next_ch)| *next_ch) == Some('\'') {
iter.next();
continue;
}
quote = None;
}
continue;
@@ -1538,6 +1660,80 @@ mod tests {
assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation));
}
#[test]
fn reports_aggregate_and_grouped_queries_per_issue_11672() {
let result = analyze_editable_query_editability("select count(*) from users");
assert!(!result.editable);
assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation));
let sql = "SELECT \n R.P_ID, \n R.SUBJECT, \n COUNT(0) AS CNT_NUM\n FROM COMMON.RECORD R\n WHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY\n AND R.SUBJECT = '邮件提醒'\n AND R.P_ID = '123214512'\n GROUP BY R.P_ID, R.SUBJECT";
let result = analyze_editable_query_editability(sql);
assert!(!result.editable);
assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation));
let with_doubled_quote = "SELECT id, name FROM users WHERE name = 'don''t stop' GROUP BY id, name";
let result = analyze_editable_query_editability(with_doubled_quote);
assert!(!result.editable);
assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation));
let comment_in_string = "SELECT id, name FROM users WHERE name = 'hello -- world' AND active = 1";
let result = analyze_editable_query_editability(comment_in_string);
assert!(result.editable);
}
#[test]
fn keeps_scalar_subquery_aggregates_separate_from_outer_rows() {
for expression in [
"(SELECT COUNT(*) FROM orders)",
"(SELECT COUNT(*) FROM orders o WHERE o.user_id = u.id)",
"COALESCE((SELECT SUM(amount) FROM orders o WHERE o.user_id = u.id), 0)",
"((SELECT COUNT(*) FROM orders))",
"(SELECT COUNT(*) FROM orders UNION SELECT COUNT(*) FROM archived_orders)",
"(SELECT MAX(amount) FROM orders WHERE amount > (SELECT AVG(amount) FROM orders))",
"CASE WHEN (SELECT COUNT(*) FROM orders) > 0 THEN 1 ELSE 0 END",
"(SELECT COUNT(*) FROM orders WHERE note = 'don''t stop')",
] {
let result =
analyze_editable_query_editability(&format!("SELECT u.id, u.name, {expression} AS total FROM users u"));
assert!(result.editable, "{expression}: {:?}", result.reason);
let analysis = result.analysis.unwrap();
assert_eq!(analysis.table_name, "users");
assert_eq!(analysis.columns[0].source_name.as_deref(), Some("id"));
assert_eq!(analysis.columns[1].source_name.as_deref(), Some("name"));
assert!(analysis.columns[2].source_name.is_none());
}
}
#[test]
fn rejects_outer_aggregates_inside_scalar_expressions() {
for expression in [
"COUNT(*)",
"COALESCE(SUM(amount), 0)",
"ABS(MAX(amount)) + 1",
"(SUM(amount))",
"COALESCE((SELECT COUNT(*) FROM orders), SUM(amount))",
"SUM((SELECT COUNT(*) FROM orders))",
"CASE WHEN amount > 0 THEN SUM(amount) ELSE 0 END",
] {
let result = analyze_editable_query_editability(&format!("SELECT {expression} AS total FROM users"));
assert!(!result.editable, "{expression}");
assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation), "{expression}");
}
for sql in ["SELECT name FROM users GROUP BY name", "SELECT name FROM users HAVING COUNT(*) > 1"] {
let result = analyze_editable_query_editability(sql);
assert!(!result.editable, "{sql}");
assert_eq!(result.reason, Some(QueryEditabilityReason::Aggregation));
}
for sql in [
"SELECT DISTINCT id, name FROM users",
"SELECT id, name, 'COUNT(*)' AS note FROM users",
"SELECT id, \"COUNT\", name FROM users",
"SELECT id, name FROM users /* GROUP BY name */",
] {
assert!(analyze_editable_query_editability(sql).editable, "{sql}");
}
}
#[test]
fn keeps_single_table_expression_columns() {
let result = analyze_editable_query_editability(
packages/app-tests/sqlAnalysis.test.ts
Vendored
+99 -8
View File
@@ -1,6 +1,17 @@
import { strict as assert } from "node:assert";
import { test } from "vitest";
import { allEditableColumnsWriteable, allPrimaryKeysPresent, analyzeEditableQuery, analyzeEditableQueryEditability, analyzeSelectStructureForDisplay, isBinaryType, queryEditabilityMessageKey, resolveMetadataColumnName, sourceColumnsForResult } from "../../apps/desktop/src/lib/sql/sqlAnalysis.ts";
import {
allEditableColumnsWriteable,
allPrimaryKeysPresent,
analyzeEditableQuery,
analyzeEditableQueryEditability,
analyzeSelectStructureForDisplay,
hasTopLevelKeyword,
isBinaryType,
queryEditabilityMessageKey,
resolveMetadataColumnName,
sourceColumnsForResult,
} from "../../apps/desktop/src/lib/sql/sqlAnalysis.ts";
test("recognizes a simple single-table SELECT as editable", () => {
const result = analyzeEditableQueryEditability("select id, name from public.users where active = true order by id");
@@ -354,13 +365,10 @@ test("does not bind non-Oracle same-arity label mismatches by projection order",
test("keeps Oracle queries with a NUM column or alias editable when result labels disagree", () => {
const byName = analyzeEditableQuery("select id, num from users");
assert.ok(byName);
assert.deepEqual(
byName.columns,
[
{ sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" },
{ sourceName: "num", sourceNameQuoted: false, resultName: "num", expression: "num" },
],
);
assert.deepEqual(byName.columns, [
{ sourceName: "id", sourceNameQuoted: false, resultName: "id", expression: "id" },
{ sourceName: "num", sourceNameQuoted: false, resultName: "num", expression: "num" },
]);
assert.deepEqual(sourceColumnsForResult(byName, ["ID", "NUM"], undefined, "oracle"), ["id", "num"]);
assert.equal(allEditableColumnsWriteable(byName, ["ID", "NUM"], undefined, "oracle"), true);
assert.deepEqual(sourceColumnsForResult(byName, ["ID", "ROWNUM"], undefined, "oracle"), ["id", undefined]);
@@ -411,3 +419,86 @@ test("recognizes binary type declarations with lengths", () => {
assert.equal(isBinaryType("VARBINARY(255)"), true);
assert.equal(isBinaryType("varchar(255)"), false);
});
test("rejects aggregate queries with aggregation reason (#11672)", () => {
assert.deepEqual(analyzeEditableQueryEditability("SELECT COUNT(*) FROM users"), { editable: false, reason: "aggregation" });
assert.deepEqual(analyzeEditableQueryEditability("SELECT id, COUNT(0) AS cnt FROM users"), { editable: false, reason: "aggregation" });
assert.deepEqual(analyzeEditableQueryEditability("SELECT id, SUM(amount) AS total FROM orders GROUP BY id"), { editable: false, reason: "aggregation" });
assert.deepEqual(
analyzeEditableQueryEditability(`SELECT
R.P_ID,
R.SUBJECT,
COUNT(0) AS CNT_NUM
FROM COMMON.RECORD R
WHERE R.CREATE_TIME >= SYSDATE - INTERVAL '1' DAY
AND R.SUBJECT = '邮件提醒'
AND R.P_ID = '123214512'
GROUP BY R.P_ID, R.SUBJECT`),
{ editable: false, reason: "aggregation" },
);
});
test("handles comments and escaped quotes without quote desync (#11672)", () => {
// String literal containing comment markers
const withCommentInString = analyzeEditableQueryEditability("SELECT id, name FROM users WHERE name = 'hello -- world' AND active = 1");
assert.equal(withCommentInString.editable, true);
// Doubled single quote inside string literal
const withDoubledQuote = analyzeEditableQueryEditability("SELECT id, name FROM users WHERE name = 'don''t stop' GROUP BY id, name");
assert.deepEqual(withDoubledQuote, { editable: false, reason: "aggregation" });
});
test("keeps scalar-subquery aggregates separate from editable outer rows", () => {
for (const expression of [
"(SELECT COUNT(*) FROM orders)",
"(SELECT COUNT(*) FROM orders o WHERE o.user_id = u.id)",
"COALESCE((SELECT SUM(amount) FROM orders o WHERE o.user_id = u.id), 0)",
"((SELECT COUNT(*) FROM orders))",
"(SELECT COUNT(*) FROM orders UNION SELECT COUNT(*) FROM archived_orders)",
"(SELECT MAX(amount) FROM orders WHERE amount > (SELECT AVG(amount) FROM orders))",
"CASE WHEN (SELECT COUNT(*) FROM orders) > 0 THEN 1 ELSE 0 END",
"(SELECT COUNT(*) FROM orders WHERE note = 'don''t stop')",
]) {
const result = analyzeEditableQueryEditability(`SELECT u.id, u.name, ${expression} AS total FROM users u`);
assert.equal(result.editable, true, expression);
if (!result.editable) continue;
assert.deepEqual(
result.analysis.columns.map((column) => column.sourceName),
["id", "name", undefined],
);
assert.equal(result.analysis.tableName, "users");
}
});
test("rejects outer aggregates inside scalar expressions after nested queries", () => {
for (const expression of ["COUNT(*)", "COALESCE(SUM(amount), 0)", "ABS(MAX(amount)) + 1", "(SUM(amount))", "COALESCE((SELECT COUNT(*) FROM orders), SUM(amount))", "SUM((SELECT COUNT(*) FROM orders))", "CASE WHEN amount > 0 THEN SUM(amount) ELSE 0 END"]) {
assert.deepEqual(analyzeEditableQueryEditability(`SELECT ${expression} AS total FROM users`), { editable: false, reason: "aggregation" }, expression);
}
assert.deepEqual(analyzeEditableQueryEditability("SELECT name FROM users GROUP BY name"), { editable: false, reason: "aggregation" });
assert.deepEqual(analyzeEditableQueryEditability("SELECT name FROM users HAVING COUNT(*) > 1"), { editable: false, reason: "aggregation" });
assert.equal(analyzeEditableQueryEditability("SELECT DISTINCT id, name FROM users").editable, true);
assert.equal(analyzeEditableQueryEditability("SELECT id, name, 'COUNT(*)' AS note FROM users").editable, true);
assert.equal(analyzeEditableQueryEditability('SELECT id, "COUNT", name FROM users').editable, true);
});
test("scans grouping keywords with dialect-aware comments and quotes", () => {
const keywords = ["GROUP", "HAVING"];
for (const sql of [
"SELECT name FROM users /* GROUP BY name HAVING COUNT(*) > 1 */",
"SELECT name FROM users -- GROUP BY name\nWHERE active = 1",
"SELECT name, 'GROUP BY name HAVING count(*)' AS note FROM users",
'SELECT "GROUP", "HAVING", name FROM users',
"SELECT name FROM users WHERE id IN (SELECT id FROM orders GROUP BY id HAVING COUNT(*) > 1)",
]) {
assert.equal(hasTopLevelKeyword(sql, keywords, "oracle"), false, sql);
}
assert.equal(hasTopLevelKeyword("SELECT name, q'[don't GROUP BY name]' AS note FROM users", keywords, "oracle"), false);
assert.equal(hasTopLevelKeyword("SELECT name, $$GROUP BY name HAVING COUNT(*)$$ AS note FROM users", keywords, "postgres"), false);
assert.equal(hasTopLevelKeyword("SELECT name FROM users # GROUP BY name", keywords, "mysql"), false);
assert.equal(hasTopLevelKeyword("SELECT `GROUP`, `HAVING` FROM users", keywords, "mysql"), false);
assert.equal(hasTopLevelKeyword("SELECT [GROUP], [HAVING] FROM users", keywords, "sqlserver"), false);
assert.equal(hasTopLevelKeyword("SELECT name FROM users WHERE note = 'don\\'t GROUP BY name'", keywords, "mysql"), false);
assert.equal(hasTopLevelKeyword("SELECT name FROM users WHERE amount = 1--1 GROUP BY name", keywords, "mysql"), true);
assert.equal(hasTopLevelKeyword("SELECT name FROM users /* ignored */ GROUP BY name", keywords, "oracle"), true);
assert.equal(hasTopLevelKeyword("SELECT name FROM users HAVING COUNT(*) > 1", keywords, "oracle"), true);
});