diff --git a/frontend/src/metabase-lib/Question.ts b/frontend/src/metabase-lib/Question.ts index 0bce05563fae457bdada61f964beae694cfcb00c..c2fc003715d255058c01c84ff5efef67b8397fcf 100644 --- a/frontend/src/metabase-lib/Question.ts +++ b/frontend/src/metabase-lib/Question.ts @@ -23,18 +23,18 @@ import { sortObject } from "metabase-lib/utils"; import type { Card as CardObject, + CardDisplayType, CardType, CollectionId, DatabaseId, - DatasetQuery, + Dataset, DatasetData, - TableId, + DatasetQuery, Parameter as ParameterObject, - ParameterValues, ParameterId, + ParameterValues, + TableId, VisualizationSettings, - CardDisplayType, - Dataset, } from "metabase-types/api"; // TODO: remove these dependencies @@ -720,13 +720,19 @@ class Question { databaseId(): DatabaseId | null { const query = this.query(); - const databaseId = Lib.databaseID(query); - return databaseId; + return Lib.databaseID(query); } legacyQueryTable(): Table | null { - const query = this.legacyQuery({ useStructuredQuery: true }); - return query && typeof query.table === "function" ? query.table() : null; + const query = this.query(); + const { isNative } = Lib.queryDisplayInfo(query); + if (isNative) { + return this.legacyQuery().table(); + } else { + const tableId = Lib.sourceTableOrCardId(query); + const metadata = this.metadata(); + return metadata.table(tableId); + } } legacyQueryTableId(): TableId | null { diff --git a/frontend/src/metabase-lib/queries/StructuredQuery.ts b/frontend/src/metabase-lib/queries/StructuredQuery.ts index eeba097194119246d9051aa2ceccc19a900b57e1..613ce905b710677fa69f3478fbc24e90b0411f64 100644 --- a/frontend/src/metabase-lib/queries/StructuredQuery.ts +++ b/frontend/src/metabase-lib/queries/StructuredQuery.ts @@ -52,8 +52,6 @@ import AggregationWrapper from "./structured/Aggregation"; import BreakoutWrapper from "./structured/Breakout"; import FilterWrapper from "./structured/Filter"; -import { getStructuredQueryTable } from "./utils/structured-query-table"; - type DimensionFilterFn = (dimension: Dimension) => boolean; export type FieldFilterFn = (filter: Field) => boolean; @@ -195,7 +193,9 @@ class StructuredQuery extends AtomicQuery { * @returns the table object, if a table is selected and loaded. */ table = _.once((): Table | null => { - return getStructuredQueryTable(this.question(), this); + const question = this.question(); + const metadata = question.metadata(); + return metadata.table(this._sourceTableId()); }); hasAggregations() { diff --git a/frontend/src/metabase-lib/queries/utils/native-query-table.ts b/frontend/src/metabase-lib/queries/utils/native-query-table.ts index 07066cfdca617560b52afccb6a91682d5bcede6b..256d7285842291662fd5a7a4360b70405dc5267a 100644 --- a/frontend/src/metabase-lib/queries/utils/native-query-table.ts +++ b/frontend/src/metabase-lib/queries/utils/native-query-table.ts @@ -1,16 +1,16 @@ import _ from "underscore"; import type Table from "metabase-lib/metadata/Table"; +import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions"; import type NativeQuery from "../NativeQuery"; -import { getDatasetTable } from "./nested-card-query-table"; export function getNativeQueryTable(nativeQuery: NativeQuery): Table | null { const question = nativeQuery.question(); const isDataset = question.isDataset() && question.isSaved(); if (isDataset) { - return getDatasetTable(nativeQuery); + return question.metadata().table(getQuestionVirtualTableId(question.id())); } const database = question.database(); diff --git a/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts b/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts deleted file mode 100644 index a13c00876dea67d36bcb0547f1655ac8b3c5a7b2..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts +++ /dev/null @@ -1,72 +0,0 @@ -import * as Lib from "metabase-lib"; -import { - getQuestionIdFromVirtualTableId, - getQuestionVirtualTableId, -} from "metabase-lib/metadata/utils/saved-questions"; -import type Table from "metabase-lib/metadata/Table"; -import type Question from "metabase-lib/Question"; -import type StructuredQuery from "../StructuredQuery"; -import type NativeQuery from "../NativeQuery"; -import { createVirtualField, createVirtualTable } from "./virtual-table"; - -// This function expects a `sourceTableId` to exist in the `metadata.table` cache -// It also expects the card associated with the `sourceTableId` to exist in the `metadata.question` cache -export function getNestedCardTable(question: Question): Table | null { - const query = question.query(); - const metadata = question.metadata(); - const sourceTableId = Lib.sourceTableOrCardId(query); - const nestedCardTable = metadata.table(sourceTableId); - if (nestedCardTable) { - return nestedCardTable; - } - - const questionId = getQuestionIdFromVirtualTableId(sourceTableId); - const nestedQuestion = metadata.question(questionId); - // There are scenarios (and possible race conditions) in the application where - // the nested card table might not be available, but if we have access to a Question - // with result_metadata then we might as well use it to create virtual fields - if (nestedQuestion) { - return createVirtualTableUsingQuestionMetadata(nestedQuestion); - } - - return null; -} - -// Treat the Dataset/Model like a Question that uses itself as its source table -// Expects the Question to have been fetched as a virtual table -export function getDatasetTable( - legacyQuery: StructuredQuery | NativeQuery, -): Table | null { - const question = legacyQuery.question(); - const composedDatasetQuestion = question.composeDataset(); - return getNestedCardTable(composedDatasetQuestion); -} - -function createVirtualTableUsingQuestionMetadata(question: Question): Table { - const metadata = question.metadata(); - const questionResultMetadata = question.getResultMetadata(); - const questionDisplayName = question.displayName() as string; - const legacyQuery = question.legacyQuery({ useStructuredQuery: true }) as - | StructuredQuery - | NativeQuery; - const fields = questionResultMetadata.map((fieldMetadata: any) => { - const field = metadata.field(fieldMetadata.id); - const virtualField = field - ? field.clone(fieldMetadata) - : createVirtualField(fieldMetadata); - - virtualField.query = legacyQuery; - virtualField.metadata = metadata; - - return virtualField; - }); - - return createVirtualTable({ - id: getQuestionVirtualTableId(question.id()), - name: questionDisplayName, - display_name: questionDisplayName, - db: question?.database() ?? undefined, - fields, - metadata, - }); -} diff --git a/frontend/src/metabase-lib/queries/utils/structured-query-table.ts b/frontend/src/metabase-lib/queries/utils/structured-query-table.ts deleted file mode 100644 index b572476729d9f52739cf450ca7dd2f9a41ae3465..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-lib/queries/utils/structured-query-table.ts +++ /dev/null @@ -1,89 +0,0 @@ -import type { FieldReference } from "metabase-types/api"; -import * as Lib from "metabase-lib"; -import { isVirtualCardId } from "metabase-lib/metadata/utils/saved-questions"; -import type Field from "metabase-lib/metadata/Field"; -import type Table from "metabase-lib/metadata/Table"; -import type Question from "metabase-lib/Question"; - -import type StructuredQuery from "../StructuredQuery"; -import { createVirtualTable, createVirtualField } from "./virtual-table"; -import { getDatasetTable, getNestedCardTable } from "./nested-card-query-table"; - -export function getStructuredQueryTable( - question: Question, - legacyQuery: StructuredQuery, -): Table | null { - const sourceQuery = legacyQuery.sourceQuery(); - // 1. Query has a source query. Use the source query as a table. - if (sourceQuery) { - return getSourceQueryTable(question, legacyQuery); - } - - // 2. Query has a source table that is a nested card. - const query = question.query(); - const sourceTableId = Lib.sourceTableOrCardId(query); - if (isVirtualCardId(sourceTableId)) { - return getNestedCardTable(question); - } - - // 3. The query's question is a saved dataset. - const isDataset = question.isDataset() && question.isSaved(); - if (isDataset) { - return getDatasetTable(legacyQuery); - } - - // 4. The query's table is a concrete table, assuming one exists in `metadata`. - // Failure to find a table at this point indicates that there is a bug. - return legacyQuery.metadata().table(sourceTableId); -} - -function getFieldsForSourceQueryTable( - originalQuery: StructuredQuery, - sourceQuery: StructuredQuery, -): Field[] { - const metadata = originalQuery.metadata(); - return sourceQuery.columns().map(column => { - // Not sure why we build out `id` like this, but it's what the old code did - const id: FieldReference = [ - "field", - column.name, - { - "base-type": column.base_type as string, - }, - ]; - - const virtualField = createVirtualField({ - ...column, - id, - source: "fields", - query: originalQuery, - metadata, - }); - - return virtualField; - }); -} - -function getSourceQueryTable( - question: Question, - legacyQuery: StructuredQuery, -): Table | null { - const sourceQuery = legacyQuery.sourceQuery() as StructuredQuery; - const fields = getFieldsForSourceQueryTable(legacyQuery, sourceQuery); - const query = question.query(); - const sourceTableId = Lib.sourceTableOrCardId(query); - - if (!sourceTableId) { - return null; - } - - return createVirtualTable({ - id: sourceTableId, - db: question.database() ?? undefined, - fields, - metadata: sourceQuery.metadata(), - // intentionally set these to "" so that we fallback to a title of "Previous results" in join steps - display_name: "", - name: "", - }); -} diff --git a/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts b/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts deleted file mode 100644 index 56218f12bd2844eb21174bc0a4a2c5dcfce034e8..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts +++ /dev/null @@ -1,193 +0,0 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-nocheck -import _ from "underscore"; -import { createMockMetadata } from "__support__/metadata"; -import { checkNotNull } from "metabase/lib/types"; -import { - createMockField, - createMockSavedQuestionsDatabase, - createMockStructuredDatasetQuery, - createMockStructuredQuery, - createMockTable, -} from "metabase-types/api/mocks"; -import { - createSampleDatabase, - createSavedStructuredCard, - createStructuredModelCard, - createProductsIdField, - ORDERS_ID, - PRODUCTS_ID, - SAMPLE_DB_ID, -} from "metabase-types/api/mocks/presets"; -import Table from "metabase-lib/metadata/Table"; -import type Field from "metabase-lib/metadata/Field"; -import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions"; - -import { getStructuredQueryTable } from "./structured-query-table"; - -const SAVED_QUESTIONS_DB = createMockSavedQuestionsDatabase(); - -const modelField = createMockField({ - ...createProductsIdField(), - display_name: "~*~Products.ID~*~", -}); - -const modelCard = createStructuredModelCard({ - id: 1, - name: "Structured Model", - result_metadata: [modelField], -}); -const modelTable = createMockTable({ - id: getQuestionVirtualTableId(modelCard.id), - db_id: SAVED_QUESTIONS_DB.id, - name: modelCard.name, - display_name: modelCard.name, - fields: [modelField], -}); - -const card = createSavedStructuredCard({ id: 2, name: "Base question" }); -const cardTable = createMockTable({ - id: getQuestionVirtualTableId(card.id), - db_id: SAVED_QUESTIONS_DB.id, - name: card.name, - display_name: card.name, - fields: [createMockField({ table_id: getQuestionVirtualTableId(card.id) })], -}); - -const nestedCard = createSavedStructuredCard({ - id: 3, - name: "Question based on another question", - dataset_query: createMockStructuredDatasetQuery({ - database: SAMPLE_DB_ID, - query: createMockStructuredQuery({ - "source-table": getQuestionVirtualTableId(card.id), - }), - }), -}); - -const sourceQueryCard = createSavedStructuredCard({ - id: 4, - name: "Question based on a source query", - dataset_query: createMockStructuredDatasetQuery({ - database: SAMPLE_DB_ID, - query: createMockStructuredQuery({ - "source-query": { - "source-table": PRODUCTS_ID, - }, - }), - }), -}); - -const metadata = createMockMetadata({ - databases: [createSampleDatabase(), SAVED_QUESTIONS_DB], - tables: [cardTable, modelTable], - questions: [card, nestedCard, modelCard, sourceQueryCard], -}); - -describe("metabase-lib/queries/utils/structured-query-table", () => { - describe("Card that relies on another card as its source table", () => { - const nestedQuestion = checkNotNull(metadata.question(nestedCard.id)); - const virtualTable = checkNotNull(metadata.table(cardTable.id)); - - const table = getStructuredQueryTable( - nestedQuestion, - nestedQuestion.legacyQuery({ useStructuredQuery: true }), - ); - - it("should return a table", () => { - expect(table).toBeInstanceOf(Table); - }); - - it("should return a virtual table based on the nested card", () => { - expect(table?.getPlainObject()).toEqual(virtualTable.getPlainObject()); - expect(table?.fields).toEqual(virtualTable.fields); - }); - }); - - describe("Model card", () => { - const model = checkNotNull(metadata.question(modelCard.id)); - const table = getStructuredQueryTable( - model, - model.legacyQuery({ useStructuredQuery: true }), - ); - - it("should return a nested card table using the given query's question", () => { - expect(table?.getPlainObject()).toEqual( - expect.objectContaining({ - id: modelTable.id, - name: modelTable.name, - display_name: modelTable.display_name, - }), - ); - - const [field] = table?.fields || []; - expect(field.getPlainObject()).toEqual( - expect.objectContaining({ - ...modelField, - display_name: "~*~Products.ID~*~", - }), - ); - }); - }); - - describe("Card that relies on a source query", () => { - const sourceQueryQuestion = checkNotNull( - metadata.question(sourceQueryCard.id), - ); - const productsTable = checkNotNull(metadata.table(PRODUCTS_ID)); - - const table = getStructuredQueryTable( - sourceQueryQuestion, - sourceQueryQuestion.legacyQuery({ useStructuredQuery: true }), - ); - - it("should return a virtual table based on the nested query", () => { - expect(table?.getPlainObject()).toEqual({ - id: PRODUCTS_ID, - db_id: SAMPLE_DB_ID, - name: "", - display_name: "", - schema: "", - description: null, - active: true, - visibility_type: null, - field_order: "database", - initial_sync_status: "complete", - }); - }); - - it("should contain fields", () => { - const fields = _.sortBy( - (table?.fields || []).map(field => field.getPlainObject()), - "name", - ); - - const nestedQueryProductFields = _.sortBy( - productsTable.fields.map((field: Field) => { - const column = field.dimension().column(); - return { - ...column, - id: ["field", column.name, { "base-type": column.base_type }], - source: "fields", - }; - }), - "name", - ); - - expect(fields).toEqual(nestedQueryProductFields); - }); - }); - - describe("Card that has a concrete source table", () => { - const ordersTable = checkNotNull(metadata.table(ORDERS_ID)); - - const table = getStructuredQueryTable( - ordersTable.legacyQuery({ useStructuredQuery: true }).question(), - ordersTable.legacyQuery({ useStructuredQuery: true }), - ); - - it("should return the concrete table stored on the Metadata object", () => { - expect(table).toBe(ordersTable); - }); - }); -}); diff --git a/frontend/src/metabase-lib/queries/utils/virtual-table.ts b/frontend/src/metabase-lib/queries/utils/virtual-table.ts deleted file mode 100644 index 6beed0898477fce0f5654d03e066e8f7efb3d78b..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-lib/queries/utils/virtual-table.ts +++ /dev/null @@ -1,79 +0,0 @@ -import type { TableId } from "metabase-types/api"; -import Table from "metabase-lib/metadata/Table"; -import Field from "metabase-lib/metadata/Field"; -import type Database from "metabase-lib/metadata/Database"; -import type Metadata from "metabase-lib/metadata/Metadata"; -import type StructuredQuery from "../StructuredQuery"; -import type NativeQuery from "../NativeQuery"; - -type VirtualTableProps = { - id: TableId; - name: string; - display_name: string; - metadata: Metadata; - db?: Database; - fields?: Field[]; -}; - -type VirtualFieldProps = { - metadata: Metadata; - query: StructuredQuery | NativeQuery; -} & Partial<Field>; - -// For when you have no Table -export function createVirtualTable({ - id, - name, - display_name, - metadata, - db, - fields, -}: VirtualTableProps): Table { - const table = new Table({ - id, - db_id: Number(db?.id), - name, - display_name, - schema: "", - description: null, - active: true, - visibility_type: null, - field_order: "database", - initial_sync_status: "complete", - }); - - Object.assign(table, { - fields: fields || [], - db, - metadata, - segments: [], - metrics: [], - }); - - table.metadata = metadata; - table.db = db; - table.fields = fields || []; - - table.fields.forEach(field => { - field.table = table; - field.table_id = table.id; - }); - - return table; -} - -export function createVirtualField({ - metadata, - query, - ...rest -}: VirtualFieldProps): Field { - const field = new Field({ - source: "fields", - ...rest, - }); - - field.metadata = metadata; - field.query = query; - - return field; -} diff --git a/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts b/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts deleted file mode 100644 index ae336e10f527ed30b7f30c50bee5443fed72271c..0000000000000000000000000000000000000000 --- a/frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { createMockMetadata } from "__support__/metadata"; -import { - createSampleDatabase, - PRODUCTS_ID, -} from "metabase-types/api/mocks/presets"; -import type StructuredQuery from "metabase-lib/queries/StructuredQuery"; -import Field from "metabase-lib/metadata/Field"; -import Table from "metabase-lib/metadata/Table"; - -import { createVirtualField, createVirtualTable } from "./virtual-table"; - -describe("metabase-lib/queries/utils/virtual-table", () => { - const metadata = createMockMetadata({ - databases: [createSampleDatabase()], - }); - - const productsTable = metadata.table(PRODUCTS_ID) as Table; - - const query = productsTable - .newQuestion() - .legacyQuery({ useStructuredQuery: true }) as StructuredQuery; - const field = createVirtualField({ - id: 123, - metadata, - query, - }); - - describe("createVirtualField", () => { - it("should return a new Field instance", () => { - expect(field.id).toBe(123); - expect(field).toBeInstanceOf(Field); - }); - - it("should set `metadata` and `query` on the field instance but not its underlying plain object", () => { - expect(field.metadata).toBe(metadata); - expect(field.query).toBe(query); - - const plainObject = field.getPlainObject() as any; - expect(plainObject.metadata).toBeUndefined(); - expect(plainObject.query).toBeUndefined(); - }); - }); - - describe("createVirtualTable", () => { - const query = productsTable - .newQuestion() - .legacyQuery({ useStructuredQuery: true }) as StructuredQuery; - const field1 = createVirtualField({ - id: 1, - metadata, - query, - }); - const field2 = createVirtualField({ - id: 2, - metadata, - query, - }); - - const table = createVirtualTable({ - id: 456, - metadata, - fields: [field1, field2], - name: "", - display_name: "", - }); - - it("should return a new Table instance", () => { - expect(table.id).toBe(456); - expect(table).toBeInstanceOf(Table); - }); - - it("should set `metadata` on the table instance but not its underlying plain object", () => { - expect(table.metadata).toBe(metadata); - - const plainObject = table.getPlainObject() as any; - expect(plainObject.metadata).toBeUndefined(); - }); - - it("should add a table reference to its fields", () => { - expect(table.fields?.every(field => field.table === table)).toBe(true); - expect(table.fields?.every(field => field.table_id === table.id)).toBe( - true, - ); - }); - }); -}); diff --git a/frontend/src/metabase/query_builder/components/dataref/QuestionPane/QuestionPane.tsx b/frontend/src/metabase/query_builder/components/dataref/QuestionPane/QuestionPane.tsx index 3f625fee3359492bb5ecd6283751e372830e2ad7..239d3ebe075d49218c0172cb85475aa6a0d31ee1 100644 --- a/frontend/src/metabase/query_builder/components/dataref/QuestionPane/QuestionPane.tsx +++ b/frontend/src/metabase/query_builder/components/dataref/QuestionPane/QuestionPane.tsx @@ -7,6 +7,7 @@ import { EmptyDescription, } from "metabase/components/MetadataInfo/MetadataInfo.styled"; import Collections from "metabase/entities/collections"; +import Tables from "metabase/entities/tables"; import Questions from "metabase/entities/questions"; import SidebarContent from "metabase/query_builder/components/SidebarContent"; import type { Collection } from "metabase-types/api/collection"; @@ -14,6 +15,7 @@ import type { State } from "metabase-types/store"; import type Table from "metabase-lib/metadata/Table"; import type Question from "metabase-lib/Question"; import * as ML_Urls from "metabase-lib/urls"; +import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions"; import FieldList from "../FieldList"; import { PaneContent } from "../Pane.styled"; import { @@ -30,17 +32,18 @@ interface QuestionPaneProps { onBack: () => void; onClose: () => void; question: Question; + table: Table; collection: Collection | null; } const QuestionPane = ({ onItemClick, question, + table, collection, onBack, onClose, }: QuestionPaneProps) => { - const table = question.composeThisQuery()?.legacyQueryTable() as Table; // ? is only needed to satisfy type-checker return ( <SidebarContent title={question.displayName() || undefined} @@ -106,6 +109,12 @@ export default _.compose( Questions.load({ id: (_state: State, props: QuestionPaneProps) => props.question.id, }), + Tables.load({ + id: (_state: State, props: QuestionPaneProps) => + getQuestionVirtualTableId(props.question.id()), + fetchType: "fetchMetadata", + requestType: "fetchMetadata", + }), Collections.load({ id: (_state: State, props: QuestionPaneProps) => props.question.collectionId(), diff --git a/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js b/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js index 38c7cd4cdd2f5488a9c399812105de974ac2c08b..ed652d671abc89f1a8b8b7054e09df0004521a7d 100644 --- a/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js +++ b/frontend/test/metabase-lib/lib/queries/StructuredQuery-nesting.unit.spec.js @@ -67,23 +67,6 @@ describe("StructuredQuery nesting", () => { }, }); }); - - it("should return a table with correct dimensions", () => { - const { ordersTable } = setup(); - const q = ordersTable - .legacyQuery() - .aggregate(["count"]) - .breakout(["field", ORDERS.PRODUCT_ID, null]); - expect( - q - .nest() - .filterDimensionOptions() - .dimensions.map(d => d.mbql()), - ).toEqual([ - ["field", "PRODUCT_ID", { "base-type": "type/Integer" }], - ["field", "count", { "base-type": "type/Integer" }], - ]); - }); }); describe("topLevelQuery", () => {