Remove MLv1 virtual tables (#38806)

* Migrate legacyQueryTable

* Migrate legacyQueryTable

* Load missing data

* Remove virtual tables

* Remove the test

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 {

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() {

frontend/src/metabase-lib/queries/utils/native-query-table.ts

 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();

frontend/src/metabase-lib/queries/utils/nested-card-query-table.ts

-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,
-  });
-}

frontend/src/metabase-lib/queries/utils/structured-query-table.ts

-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: "",
-  });
-}

frontend/src/metabase-lib/queries/utils/structured-query-table.unit.spec.ts

-// 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);
-    });
-  });
-});

frontend/src/metabase-lib/queries/utils/virtual-table.ts

-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;
-}

frontend/src/metabase-lib/queries/utils/virtual-table.unit.spec.ts

-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,
-      );
-    });
-  });
-});

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(),

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", () => {