Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated native models may lose their result set metadata #37009

Closed
zbodi74 opened this issue Dec 20, 2023 · 1 comment · Fixed by #39201, #39311 or #40087
Closed

Updated native models may lose their result set metadata #37009

zbodi74 opened this issue Dec 20, 2023 · 1 comment · Fixed by #39201, #39311 or #40087
Assignees
Labels
.Backend .Escalation .Frontend Priority:P2 Average run of the mill bug Querying/Nested Queries Questions based on other saved questions Querying/ Type:Bug Product defects
Milestone

Comments

@zbodi74
Copy link

zbodi74 commented Dec 20, 2023

Describe the bug

Upon updating a native model which is based on another model, the result set metadata may be lost.
When this occurs, the query editor won't show available columns for questions that are based on this model.

The workaround is to update the model, and making sure to preview the results before saving it.

To Reproduce

  1. Create native model model1:
      select 1 as "client_id", 1 as "value"
UNION select 1 as "client_id", 2 as "value"
UNION select 2 as "client_id", 3 as "value"
UNION select 3 as "client_id", 4 as "value"
  1. Create and save a native model model2, which uses model1 as its source:
select * from {{ #nnn-model1 }}
  1. Update the query definition of model2, and make a minor, non-impactful change, for instance:
select cte.* from {{ #nnn-model1 }} as cte

, and save it, without previewing the results in the editor.

  1. Now start a GUI question based on model2.
  2. (!)The columns of the model won't be available in the editor.
image

Expected behavior

It should wirk

Logs

Logs:

2023-12-20 18:47:06,445 INFO api.dataset :: Source query for this query is Card 77
2023-12-20 18:47:06,455 WARN middleware.add-implicit-clauses :: Warning: cannot determine fields for an explicit `source-query` unless you also include `source-metadata`.
Query: {:params [],
 :native
 "select cte.* from (select 1 as \"client_id\", 1 as \"value\"\nUNION select 1 as \"client_id\", 2 as \"value\"\nUNION select 2 as \"client_id\", 3 as \"value\"\nUNION select 3 as \"client_id\", 4 as \"value\") as cte"}

/api/card response for model2:

{
  "description": null,
  "archived": false,
  "collection_position": 1,
  "table_id": null,
  "result_metadata": null,
  "creator": {
[…]
  },
  "can_write": true,
  "database_id": 1,
  "enable_embedding": false,
  "collection_id": null,
  "query_type": "native",
  "name": "model2",
  "last_query_start": "2023-12-20T18:49:08.371778Z",
  "dashboard_count": 0,
  "persisted": false,
  "average_query_time": 21.0,
  "creator_id": 1,
  "moderation_reviews": [],
  "updated_at": "2023-12-20T18:47:06.210264Z",
  "made_public_by_id": null,
  "embedding_params": null,
  "cache_ttl": null,
  "dataset_query": {
    "type": "native",
    "native": {
      "query": "select cte.* from {{ #76-model1 }} as cte",
      "template-tags": {
        "#76-model1": {
          "type": "card",
          "name": "#76-model1",
          "id": "56149603-5eb3-4829-84a3-1afb2774f8ff",
          "card-id": 76,
          "display-name": "#76 Model1"
        }
      }
    },
    "database": 1
  },
  "id": 77,
  "parameter_mappings": [],
  "display": "table",
  "entity_id": "srrDqNwZugt2Ip3tYamZH",
  "collection_preview": true,
  "last-edit-info": {
[…]
  },
  "visualization_settings": {
    "table.pivot_column": "client_id",
    "table.cell_column": "value"
  },
  "collection": {
    "metabase.models.collection.root/is-root?": true,
    "authority_level": null,
    "name": "Our analytics",
    "is_personal": false,
    "id": "root",
    "can_write": true
  },
  "metabase_version": "v1.48.1 (a8302d4)",
  "parameters": [],
  "dataset": true,
  "created_at": "2023-12-20T18:45:50.916686",
  "parameter_usage_count": 0,
  "public_uuid": null
}```

### Information about your Metabase installation

```JSON
1.48.0, 1.47.9

Severity

Found this while troubleshooting for a customer. A simple workaround exists.

Additional context

No response

@zbodi74 zbodi74 added Type:Bug Product defects Querying/Nested Queries Questions based on other saved questions .Needs Triage Querying/ labels Dec 20, 2023
@zbodi74 zbodi74 changed the title Updated native models may lose their result set metadata and dependent queries may break Updated native models may lose their result set metadata Dec 20, 2023
@paoliniluis paoliniluis added Priority:P2 Average run of the mill bug and removed .Needs Triage labels Dec 21, 2023
metamben added a commit that referenced this issue Feb 26, 2024
metabase-bot bot added a commit that referenced this issue Feb 27, 2024
…9207)

* Keep result-metadata of native queries if possible (#39184)

Part of #37009.

* qp/query->expected-cols -> qp.preprocess/query->expected-cols

---------

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>
Co-authored-by: Tamás Benkő <tamas@metabase.com>
kamilmielnik added a commit that referenced this issue Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Tamás Benkő <tamas@metabase.com>
@kamilmielnik kamilmielnik added this to the 0.49 milestone Feb 28, 2024
kamilmielnik added a commit that referenced this issue Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Tamás Benkő <tamas@metabase.com>
kamilmielnik added a commit that referenced this issue Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Tamás Benkő <tamas@metabase.com>
kamilmielnik added a commit that referenced this issue Feb 28, 2024
* Keep result-metadata of native queries if possible

Part of #37009.

* Fix typo

* Add a fallback for when resultMetadata is not available

* Revert "Add a fallback for when resultMetadata is not available"

This reverts commit 5ad3a8b.

* Move resultsMetadata logic into getSubmittableQuestion

* Add a fallback for when resultMetadata is not available

* Add explanatory comment

* Guard against missing resultsMetadata

* Add repro for #35039

* Simplify the test

* Add an explanatory comment

* Fix test

* Rename promise to runQuestionPromise

---------

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>
Co-authored-by: Tamás Benkő <tamas@metabase.com>
@ranquild ranquild self-assigned this Feb 29, 2024
@kamilmielnik
Copy link
Contributor

Reopening. Fix will be reverted. See Slack discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment