mirror of
https://github.com/t8y2/dbx.git
synced 2026-10-23 11:36:21 +00:00
fix(oceanbase-oracle): use synthetic ROWID in editable query results (#11371)
This commit is contained in:
efeire
@@ -249,7 +249,7 @@ describe("editable query hidden primary keys", () => {
|
||||
expect(sourceColumnsForResult(aliased.analysis, ["ID", "NUM", "ROWNUM"], undefined, "oracle")).toEqual(["id", "amount", undefined]);
|
||||
});
|
||||
|
||||
it("maps a qualified Oracle ROWID projection to the synthetic row key", () => {
|
||||
it.each(["oracle", "oceanbase-oracle"] as const)("maps a qualified %s ROWID projection to the synthetic row key", (databaseType) => {
|
||||
const analysis: EditableQueryInfo = {
|
||||
schema: "APP",
|
||||
tableName: "USERS",
|
||||
@@ -261,9 +261,9 @@ describe("editable query hidden primary keys", () => {
|
||||
],
|
||||
};
|
||||
|
||||
expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "t:0", "oracle")).toBe(true);
|
||||
expect(sourceColumnsForResult(analysis, ["ROWID", "NAME", "LABEL"], "t:0", "oracle", ["__DBX_ROWID"])).toEqual(["__DBX_ROWID", "NAME", undefined]);
|
||||
expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "o:1", "oracle")).toBe(false);
|
||||
expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "t:0", databaseType)).toBe(true);
|
||||
expect(sourceColumnsForResult(analysis, ["ROWID", "NAME", "LABEL"], "t:0", databaseType, ["__DBX_ROWID"])).toEqual(["__DBX_ROWID", "NAME", undefined]);
|
||||
expect(allPrimaryKeysPresent(["__DBX_ROWID"], ["ROWID", "NAME", "LABEL"], analysis, "o:1", databaseType)).toBe(false);
|
||||
});
|
||||
|
||||
it("does not assign an unqualified multi-source ROWID to a target table", () => {
|
||||
|
||||
@@ -897,7 +897,7 @@ export function allPrimaryKeysPresent(primaryKeys: string[], resultColumns: stri
|
||||
analysis.columns.flatMap((column) => {
|
||||
if (!column.sourceName) return [];
|
||||
if (sourceKey && column.sourceKey !== sourceKey) return [];
|
||||
if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) return [DBX_ROWID_COLUMN, column.sourceName];
|
||||
if ((databaseType === "oracle" || databaseType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) return [DBX_ROWID_COLUMN, column.sourceName];
|
||||
return [column.sourceName];
|
||||
}),
|
||||
);
|
||||
@@ -981,7 +981,7 @@ export function sourceColumnsForResult(analysis: EditableQueryInfo, resultColumn
|
||||
return matchedColumns.map((column) => {
|
||||
if (!column) return undefined;
|
||||
if (sourceKey && column.sourceKey !== sourceKey) return undefined;
|
||||
if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) {
|
||||
if ((databaseType === "oracle" || databaseType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID" && column.sourceKey === sourceKey) {
|
||||
return primaryKeys?.length === 1 && primaryKeys[0] === DBX_ROWID_COLUMN ? DBX_ROWID_COLUMN : undefined;
|
||||
}
|
||||
return column.sourceName;
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
import { createPinia, setActivePinia } from "pinia";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
executeMulti: vi.fn(),
|
||||
executeQuery: vi.fn(),
|
||||
getColumns: vi.fn(),
|
||||
listIndexes: vi.fn(),
|
||||
listObjects: vi.fn(),
|
||||
listTables: vi.fn(),
|
||||
getConfig: vi.fn(),
|
||||
lookupLocalCompletionTables: vi.fn(),
|
||||
analyze: vi.fn(),
|
||||
preparePlan: vi.fn(),
|
||||
}));
|
||||
vi.stubGlobal("localStorage", { getItem: () => null, setItem: () => {}, removeItem: () => {} });
|
||||
vi.mock("@/lib/backend/api", () => ({
|
||||
analyzeEditableQueryEditability: mocks.analyze,
|
||||
executeMulti: mocks.executeMulti,
|
||||
executeQuery: mocks.executeQuery,
|
||||
getColumns: mocks.getColumns,
|
||||
listIndexes: mocks.listIndexes,
|
||||
listObjects: mocks.listObjects,
|
||||
listTables: mocks.listTables,
|
||||
prepareQueryPaginationExecutionPlan: mocks.preparePlan,
|
||||
closeClientConnectionSession: vi.fn().mockResolvedValue(undefined),
|
||||
closeQuerySession: vi.fn().mockResolvedValue(undefined),
|
||||
saveOpenTabsState: vi.fn(),
|
||||
cancelQuery: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/stores/connectionStore", () => ({
|
||||
useConnectionStore: () => ({
|
||||
ensureConnected: vi.fn().mockResolvedValue(undefined),
|
||||
getConfig: mocks.getConfig,
|
||||
lookupLocalCompletionTables: mocks.lookupLocalCompletionTables,
|
||||
recordConnectionLostError: vi.fn(),
|
||||
metadataGenerationFor: () => 0,
|
||||
}),
|
||||
}));
|
||||
vi.mock("@/stores/settingsStore", () => ({
|
||||
useSettingsStore: () => ({
|
||||
editorSettings: { pageSize: 100, autoCalculateTotalRows: false },
|
||||
}),
|
||||
}));
|
||||
|
||||
describe("OceanBase Oracle query ROWID preparation", () => {
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
setActivePinia(createPinia());
|
||||
const { clearTableMetadataCache } = await import("@/lib/metadata/tableMetadataCache");
|
||||
clearTableMetadataCache();
|
||||
mocks.getConfig.mockReturnValue({ id: "ob-1", name: "OceanBase", db_type: "oceanbase-oracle", database: "app", query_timeout_secs: 30 });
|
||||
mocks.getColumns.mockResolvedValue([{ name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, column_default: null, is_primary_key: false, extra: null }]);
|
||||
mocks.listIndexes.mockResolvedValue([]);
|
||||
mocks.listTables.mockResolvedValue([]);
|
||||
mocks.listObjects.mockResolvedValue([]);
|
||||
mocks.lookupLocalCompletionTables.mockReturnValue([{ name: "T_SIPF_DEBUG_LOG", type: "table", schema: "SIPF" }]);
|
||||
mocks.analyze.mockResolvedValue({ editable: false, reason: "complex-source" });
|
||||
mocks.preparePlan.mockImplementation(async (options) => ({ sqlToExecute: options.sql, useAgentResultSession: false }));
|
||||
mocks.executeMulti.mockResolvedValue([{ columns: ["TASKNAME"], rows: [["sample"]], affected_rows: 0, execution_time_ms: 1 }]);
|
||||
});
|
||||
|
||||
async function executedQuery(sql: string) {
|
||||
const { useQueryStore } = await import("@/stores/queryStore");
|
||||
const store = useQueryStore();
|
||||
const id = store.createTab("ob-1", "app", "Query");
|
||||
store.setAutoCommit(id, true);
|
||||
await store.executeTabSql(id, sql);
|
||||
expect(mocks.executeMulti).toHaveBeenCalled();
|
||||
return { store, id, sql: mocks.executeMulti.mock.calls[0]![2] as string };
|
||||
}
|
||||
|
||||
async function emittedSql(sql: string) {
|
||||
return (await executedQuery(sql)).sql;
|
||||
}
|
||||
|
||||
it("appends a ROWID expression to a single-column projection of a keyless table", async () => {
|
||||
const sql = await emittedSql("select a.taskname from sipf.t_sipf_debug_log a");
|
||||
expect(sql).toContain('ROWIDTOCHAR(ROWID) AS "__DBX_PK_0"');
|
||||
expect(sql).not.toContain('"__DBX_ROWID"');
|
||||
});
|
||||
|
||||
it.each(["select * from sipf.t_sipf_debug_log a", "select a.rowid, a.taskname from sipf.t_sipf_debug_log a", "select q.taskname from (select a.taskname from sipf.t_sipf_debug_log a) q"])("preserves queries not needing an extra ROWID: %s", async (sql) => {
|
||||
expect(await emittedSql(sql)).toBe(sql);
|
||||
});
|
||||
|
||||
it.each(["view", "unknown"])("does not inject ROWID for a %s source", async (type) => {
|
||||
mocks.lookupLocalCompletionTables.mockReturnValue(type === "unknown" ? [] : [{ name: "T_SIPF_DEBUG_LOG", type, schema: "SIPF" }]);
|
||||
const sql = "select a.taskname from sipf.t_sipf_debug_log a";
|
||||
expect(await emittedSql(sql)).toBe(sql);
|
||||
expect(mocks.listTables).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("prefers a physical primary key", async () => {
|
||||
mocks.getColumns.mockResolvedValue([
|
||||
{ name: "ID", data_type: "NUMBER", is_nullable: false, is_primary_key: true },
|
||||
{ name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false },
|
||||
]);
|
||||
const sql = await emittedSql("select a.taskname from sipf.t_sipf_debug_log a");
|
||||
expect(sql).toContain('"ID" AS "__DBX_PK_0"');
|
||||
expect(sql).not.toContain("ROWID");
|
||||
});
|
||||
|
||||
it("hides the projected ROWID while retaining two row identities for editing", async () => {
|
||||
const { analyzeEditableQueryEditability } = await import("@/lib/sql/sqlAnalysis");
|
||||
mocks.analyze.mockImplementation(async (sql: string) => analyzeEditableQueryEditability(sql));
|
||||
mocks.executeMulti.mockResolvedValue([
|
||||
{
|
||||
columns: ["TASKNAME", "__DBX_PK_0"],
|
||||
rows: [
|
||||
["first", "AAAPr9AAEAAAACXAAA"],
|
||||
["second", "AAAPr9AAEAAAACXAAB"],
|
||||
],
|
||||
affected_rows: 0,
|
||||
execution_time_ms: 1,
|
||||
},
|
||||
]);
|
||||
|
||||
const { store, id } = await executedQuery("select a.taskname from sipf.t_sipf_debug_log a");
|
||||
const tab = store.tabs.find((item) => item.id === id)!;
|
||||
expect(tab.result?.hidden_column_indexes).toEqual([1]);
|
||||
expect(tab.result?.rows).toEqual([
|
||||
["first", "AAAPr9AAEAAAACXAAA"],
|
||||
["second", "AAAPr9AAEAAAACXAAB"],
|
||||
]);
|
||||
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["TASKNAME", "__DBX_ROWID"]));
|
||||
expect(tab.tableMeta?.primaryKeys).toEqual(["__DBX_ROWID"]);
|
||||
expect(tab.queryEditabilityReason).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does not confuse a quoted physical rowid column with the synthetic identity", async () => {
|
||||
mocks.getColumns.mockResolvedValue([
|
||||
{ name: "rowid", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false },
|
||||
{ name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false },
|
||||
]);
|
||||
const sql = await emittedSql('select a."rowid", a.taskname from sipf.t_sipf_debug_log a');
|
||||
expect(sql).toBe('select a."rowid", a.taskname, ROWIDTOCHAR(ROWID) AS "__DBX_PK_0" from sipf.t_sipf_debug_log a');
|
||||
});
|
||||
|
||||
it("uses an explicitly selected ROWID as the row identity without hiding it", async () => {
|
||||
const { analyzeEditableQueryEditability } = await import("@/lib/sql/sqlAnalysis");
|
||||
mocks.analyze.mockImplementation(async (sql: string) => analyzeEditableQueryEditability(sql));
|
||||
mocks.executeMulti.mockResolvedValue([{ columns: ["RID", "TASKNAME"], rows: [["AAAPr9AAEAAAACXAAA", "first"]], affected_rows: 0, execution_time_ms: 1 }]);
|
||||
const originalSql = "select a.rowid as rid, a.taskname from sipf.t_sipf_debug_log a";
|
||||
const { store, id, sql } = await executedQuery(originalSql);
|
||||
expect(sql).toBe(originalSql);
|
||||
const tab = store.tabs.find((item) => item.id === id)!;
|
||||
expect(tab.result?.hidden_column_indexes ?? []).toEqual([]);
|
||||
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual(["__DBX_ROWID", "TASKNAME"]));
|
||||
expect(tab.tableMeta?.primaryKeys).toEqual(["__DBX_ROWID"]);
|
||||
});
|
||||
|
||||
it("retains the physical primary key when ROWID is also selected", async () => {
|
||||
const { analyzeEditableQueryEditability } = await import("@/lib/sql/sqlAnalysis");
|
||||
mocks.analyze.mockImplementation(async (sql: string) => analyzeEditableQueryEditability(sql));
|
||||
mocks.getColumns.mockResolvedValue([
|
||||
{ name: "ID", data_type: "NUMBER", is_nullable: false, is_primary_key: true },
|
||||
{ name: "TASKNAME", data_type: "VARCHAR2(100)", is_nullable: true, is_primary_key: false },
|
||||
]);
|
||||
mocks.executeMulti.mockResolvedValue([{ columns: ["RID", "TASKNAME", "__DBX_PK_0"], rows: [["AAAPr9AAEAAAACXAAA", "first", 1]], affected_rows: 0, execution_time_ms: 1 }]);
|
||||
const { store, id, sql } = await executedQuery("select a.rowid as rid, a.taskname from sipf.t_sipf_debug_log a");
|
||||
expect(sql).toContain('"ID" AS "__DBX_PK_0"');
|
||||
expect(sql).not.toContain("ROWIDTOCHAR");
|
||||
const tab = store.tabs.find((item) => item.id === id)!;
|
||||
await vi.waitFor(() => expect(tab.querySourceColumns).toEqual([undefined, "TASKNAME", "ID"]));
|
||||
expect(tab.tableMeta?.primaryKeys).toEqual(["ID"]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"select distinct a.taskname from sipf.t_sipf_debug_log a",
|
||||
"select a.taskname, count(*) from sipf.t_sipf_debug_log a group by a.taskname",
|
||||
"select a.taskname, b.taskname from sipf.t_sipf_debug_log a join sipf.t_sipf_debug_log b on a.taskname = b.taskname",
|
||||
"select a.taskname from sipf.t_sipf_debug_log a union select b.taskname from sipf.t_sipf_debug_log b",
|
||||
])("does not inject ROWID into an unsafe query: %s", async (sql) => {
|
||||
expect(await emittedSql(sql)).toBe(sql);
|
||||
});
|
||||
});
|
||||
@@ -902,7 +902,7 @@ function bindColumnsForSource(
|
||||
if (!column.sourceName) return column;
|
||||
if (column.sourceKey) {
|
||||
if (column.sourceKey !== source.key) return column;
|
||||
if (dbType === "oracle" && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return column;
|
||||
if ((dbType === "oracle" || dbType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return column;
|
||||
const canonicalName = resolveSourceColumnName(dbType, column.sourceName, column.sourceNameQuoted, tableColumns);
|
||||
return { ...column, sourceName: canonicalName };
|
||||
}
|
||||
@@ -6319,7 +6319,7 @@ export const useQueryStore = defineStore("query", () => {
|
||||
const selectedColumns = new Set(
|
||||
analysis.columns.flatMap((column) => {
|
||||
if (!column.sourceName || column.sourceKey !== sourceKey) return [];
|
||||
if (databaseType === "oracle" && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return [DBX_ROWID_COLUMN, column.sourceName];
|
||||
if ((databaseType === "oracle" || databaseType === "oceanbase-oracle") && !column.sourceNameQuoted && column.sourceName.toUpperCase() === "ROWID") return [DBX_ROWID_COLUMN, column.sourceName];
|
||||
return [column.sourceName];
|
||||
}),
|
||||
);
|
||||
@@ -6345,9 +6345,9 @@ export const useQueryStore = defineStore("query", () => {
|
||||
async function resolveOracleRowIdSafety(tab: QueryTab, loaded: LoadedEditableSource, databaseType: DatabaseType): Promise<boolean> {
|
||||
if (oracleRowIdIsSafeForQuery(tab, loaded)) return true;
|
||||
if (loaded.tableMeta.tableType?.trim()) return false;
|
||||
// Never enumerate an Oracle schema on the query execution path: large
|
||||
// Never enumerate an Oracle/OB schema on the query execution path: large
|
||||
// schemas can make this optional editability check take minutes (#8462).
|
||||
if (databaseType === "oracle") return false;
|
||||
if (databaseType === "oracle" || databaseType === "oceanbase-oracle") return false;
|
||||
|
||||
const connection = useConnectionStore().getConfig(tab.connectionId!);
|
||||
const schema = loaded.tableMeta.schema?.trim() || tab.schema?.trim() || connection?.default_schema?.trim() || "";
|
||||
@@ -6382,7 +6382,9 @@ export const useQueryStore = defineStore("query", () => {
|
||||
const unchanged = { sql, metadataSql: sql, hiddenPrimaryKeys: [], largeValuePreview };
|
||||
const missingPrimaryKeys =
|
||||
declaredPrimaryKeys.length === 0
|
||||
? primaryKeys.filter((primaryKey) => !(databaseType === "oracle" && primaryKey === DBX_ROWID_COLUMN && metadataAnalysis.columns.some((column) => column.sourceKey === loaded.source.key && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID")))
|
||||
? primaryKeys.filter(
|
||||
(primaryKey) => !((databaseType === "oracle" || databaseType === "oceanbase-oracle") && primaryKey === DBX_ROWID_COLUMN && metadataAnalysis.columns.some((column) => column.sourceKey === loaded.source.key && !column.sourceNameQuoted && column.sourceName?.toUpperCase() === "ROWID")),
|
||||
)
|
||||
: missingPrimaryKeysForSource(databaseType, primaryKeys, metadataAnalysis, loaded.source.key);
|
||||
if (missingPrimaryKeys.length === 0) return unchanged;
|
||||
const primaryKeySet = new Set(primaryKeys);
|
||||
@@ -6394,7 +6396,7 @@ export const useQueryStore = defineStore("query", () => {
|
||||
databaseType,
|
||||
primaryKeys: missingPrimaryKeys,
|
||||
existingResultNames: metadataAnalysis.selectStar ? loaded.tableMeta.columns.map((column) => column.name) : metadataAnalysis.columns.map((column) => column.resultName),
|
||||
sourceExpressions: missingPrimaryKeys.includes(DBX_ROWID_COLUMN) && (databaseType === "oracle" || databaseType === "xugu") ? { [DBX_ROWID_COLUMN]: databaseType === "oracle" ? "ROWIDTOCHAR(ROWID)" : "ROWID" } : undefined,
|
||||
sourceExpressions: missingPrimaryKeys.includes(DBX_ROWID_COLUMN) && (databaseType === "oracle" || databaseType === "oceanbase-oracle" || databaseType === "xugu") ? { [DBX_ROWID_COLUMN]: databaseType === "xugu" ? "ROWID" : "ROWIDTOCHAR(ROWID)" } : undefined,
|
||||
});
|
||||
if (!rewritten) return unchanged;
|
||||
queryExecutionLog("info", "hidden-primary-keys", {
|
||||
@@ -6474,9 +6476,9 @@ export const useQueryStore = defineStore("query", () => {
|
||||
if (loaded.tableMeta.columns.length === 0) return unchanged;
|
||||
if (loaded.tableMeta.tableType?.toUpperCase().includes("VIEW")) return unchanged;
|
||||
const primaryKeys = loaded.tableMeta.primaryKeys;
|
||||
const syntheticRowId = (databaseType === "oracle" || databaseType === "xugu") && usesSyntheticRowIdKey(databaseType, primaryKeys, loaded.tableMeta.tableType);
|
||||
const syntheticRowId = (databaseType === "oracle" || databaseType === "oceanbase-oracle" || databaseType === "xugu") && usesSyntheticRowIdKey(databaseType, primaryKeys, loaded.tableMeta.tableType);
|
||||
// Base tables without a natural identifier use the same ROWID identity
|
||||
// as table-data tabs (Oracle and Xugu). Confirm the object is a base
|
||||
// as table-data tabs (Oracle, OceanBase Oracle and Xugu). Confirm the object is a base
|
||||
// table because selecting ROWID from a view can fail with ORA-01445.
|
||||
if (syntheticRowId && !(await resolveOracleRowIdSafety(tab, loaded, databaseType))) return unchanged;
|
||||
const declaredPrimaryKeys = syntheticRowId ? [] : primaryKeys;
|
||||
|
||||
Reference in New Issue
Block a user