Only retains first 2 data series when saving chart with custom expressions

[landm]

Describe the bug
When saving a question, only the first 2 data series are retained in the saved version.

Logs

[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:00-06:00 INFO metabase.task.sync-databases Starting sync task for Database 2.
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:00-06:00 INFO metabase.sync.util STARTING: Sync metadata for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:00-06:00 INFO metabase.sync.util STARTING: step 'sync-timezone' for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:00-06:00 INFO metabase.task.send-pulses Sending scheduled pulses...
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:00-06:00 INFO metabase.sync.util FINISHED: step 'sync-timezone' for snowflake Database 2 'ANALYTICS' (452.6 ms)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:00-06:00 INFO metabase.sync.util STARTING: step 'sync-tables' for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:01-06:00 DEBUG metabase.server.middleware.log POST /api/dataset/pivot 202 [ASYNC: completed] 774.4 ms (25 DB calls) App DB connections: 1/15 Jetty threads: 2/50 (7 idle, 0 queued) (113 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:07-06:00 INFO metabase.api.card Card results metadata passed in to API is VALID. Thanks!
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:07-06:00 DEBUG metabase.server.middleware.log PUT /api/card/300 202 [ASYNC: completed] 104.5 ms (16 DB calls) App DB connections: 0/15 Jetty threads: 2/50 (7 idle, 0 queued) (113 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:07-06:00 DEBUG metabase.server.middleware.log GET /api/alert/question/300 200 1.7 ms (1 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (6 idle, 0 queued) (113 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:16-06:00 INFO metabase.api.card Card results metadata passed in to API is VALID. Thanks!
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:16-06:00 DEBUG metabase.server.middleware.log PUT /api/card/315 202 [ASYNC: completed] 46.9 ms (13 DB calls) App DB connections: 1/15 Jetty threads: 2/50 (7 idle, 0 queued) (113 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:00:16-06:00 DEBUG metabase.server.middleware.log GET /api/alert/question/315 200 1.6 ms (1 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (6 idle, 0 queued) (112 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:08-06:00 DEBUG metabase.server.middleware.log GET /api/user/current 200 3.2 ms (3 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (5 idle, 0 queued) (111 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:08-06:00 DEBUG metabase.server.middleware.log GET /api/session/properties 200 3.5 ms (2 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (5 idle, 0 queued) (111 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:08-06:00 DEBUG metabase.server.middleware.log GET /api/setting 200 442.3 µs (0 DB calls) App DB connections: 2/15 Jetty threads: 5/50 (3 idle, 0 queued) (112 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:08-06:00 DEBUG metabase.server.middleware.log GET /api/database 200 9.6 ms (3 DB calls) App DB connections: 0/15 Jetty threads: 4/50 (4 idle, 0 queued) (113 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:08-06:00 DEBUG metabase.server.middleware.log GET /api/session/properties 200 12.2 ms (2 DB calls) App DB connections: 0/15 Jetty threads: 5/50 (3 idle, 0 queued) (114 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:08-06:00 DEBUG metabase.server.middleware.log GET /api/setup/admin_checklist 200 10.6 ms (11 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (5 idle, 0 queued) (114 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:10-06:00 DEBUG metabase.server.middleware.log GET /api/util/bug_report_details 200 3.7 ms (1 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (5 idle, 0 queued) (114 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:11-06:00 DEBUG metabase.server.middleware.log GET /api/task/info 200 2.9 ms (0 DB calls) App DB connections: 0/15 Jetty threads: 3/50 (5 idle, 0 queued) (114 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:26-06:00 INFO metabase.sync.sync-metadata.tables Updating description for tables: (Table  'MAIN.COMBINED_LINE_ITEMS' Table  'MAIN.COMBINED_ORDERS' Table  'MAIN.NEW_SHOPIFY_ORDERS')
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:26-06:00 INFO metabase.sync.util FINISHED: step 'sync-tables' for snowflake Database 2 'ANALYTICS' (1.4 mins)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:01:26-06:00 INFO metabase.sync.util STARTING: step 'sync-fields' for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:03:51-06:00 INFO metabase.sync.util FINISHED: step 'sync-fields' for snowflake Database 2 'ANALYTICS' (2.4 mins)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:03:51-06:00 INFO metabase.sync.util STARTING: step 'sync-fks' for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:03:58-06:00 DEBUG metabase.server.middleware.log POST /api/dataset 202 [ASYNC: completed] 9.1 s (10 DB calls) App DB connections: 1/15 Jetty threads: 2/50 (5 idle, 0 queued) (113 total active threads) Queries in flight: 0 (0 queued)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:04:07-06:00 INFO metabase.sync.util FINISHED: step 'sync-fks' for snowflake Database 2 'ANALYTICS' (15.7 s)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:04:07-06:00 INFO metabase.sync.util STARTING: step 'sync-metabase-metadata' for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:04:39-06:00 INFO metabase.sync.util FINISHED: step 'sync-metabase-metadata' for snowflake Database 2 'ANALYTICS' (32.4 s)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:04:39-06:00 INFO metabase.sync.util FINISHED: Sync metadata for snowflake Database 2 'ANALYTICS' (4.7 mins)
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:04:39-06:00 INFO metabase.sync.util STARTING: Analyze data for snowflake Database 2 'ANALYTICS'
[b90c7295-fc8f-42b7-b1ee-cb3f9186f325] 2021-04-30T11:04:39-06:00 INFO metabase.sync.util STARTING: step 'fingerprint-fields' for snowflake Database 2 'ANALYTICS'

To Reproduce
Steps to reproduce the behavior:

1. Create a line chart with 3 summarization where at least 1 is a custom expression
2. Change at least one series to visualization as bar chart
3. Save the question
4. Upon saving, only the first 2 data series will remain visible in the chart

Expected behavior
Expected saved version of chart to match the pre-save version

Screenshots
Before saving:

After saving:

Information about your Metabase Installation:

{
"browser-info": {
"language": "en-US",
"platform": "MacIntel",
"userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.128 Safari/537.36",
"vendor": "Google Inc."
},
"system-info": {
"file.encoding": "UTF-8",
"java.runtime.name": "OpenJDK Runtime Environment",
"java.runtime.version": "11.0.11+9",
"java.vendor": "AdoptOpenJDK",
"java.vendor.url": "https://adoptopenjdk.net/",
"java.version": "11.0.11",
"java.vm.name": "OpenJDK 64-Bit Server VM",
"java.vm.version": "11.0.11+9",
"os.name": "Linux",
"os.version": "4.14.225-169.362.amzn2.x86_64",
"user.language": "en",
"user.timezone": "US/Pacific"
},
"metabase-info": {
"databases": [
"googleanalytics",
"snowflake"
],
"hosting-env": "elastic-beanstalk",
"application-database": "postgres",
"application-database-details": {
"database": {
"name": "PostgreSQL",
"version": "11.10"
},
"jdbc-driver": {
"name": "PostgreSQL JDBC Driver",
"version": "42.2.18"
}
},
"run-mode": "prod",
"version": {
"date": "2021-04-27",
"tag": "v0.39.1",
"branch": "release-x.39.x",
"hash": "6beba48"
},
"settings": {
"report-timezone": "US/Pacific"
}
}
}

Severity
Annoying and blocking the creation of useful dashboards or saved questions

Additional context
My guess is the issue has something to do with custom expressions as creating a chart with 3 series where none are custom expressions does not demonstrate this behavior

Issue seems to be new to release-x.39.x, reverted to 0.38.4 and did not have this issue.

Not sure if multiple visualization types matters or not, as sometimes the behavior is reproduced with all line visualizations.

⬇️ Please click the 👍 reaction instead of leaving a +1 or update? comment

[flamber]

Hi @landm
I am not able to reproduce this. Can you try reproducing with Sample Dataset?

[landm]

Yep.

1. From Sample Dataset, open Notebook Editor view
2. Create Custom Expressions:

• Formula: Sum([Discount]) / Sum([Subtotal]) | Name: Discount %
• Formula: Sum([Tax]) / Sum([Subtotal]) | Name: Tax %
• Formula: Sum([Subtotal]) / Sum([Total]) | Name: Subtotal %

3. Save Question

Pre-save:

Post-save:

[flamber]

@landm Okay, so that's a strange one. It requires just one CE, but it has to be the first metric, then only two metrics are visible, when saving. If you add the CE to the end, then it doesn't cause this problem, which is why I had problems reproducing earlier.
Regression since 0.39.0

[salsakran]

I repro'd it as well, and it's definitely a frontend bug.

The api endpoint returns all 3 aggregations.

[flamber]

To make it more clear - it only happens, when the first Y-axis series is the first CE metric.

When I create something like this, then it would only save the two first series (drops everything else in visualization_settings."graph.metrics") on Save:

While if I switch the first Y-axis series to one of the other metrics, then all series are preserved on Save:

[pawit-metabase]

This is already fixed by #16259 (since 0.40).

Root Cause
The visualization configuration is saved in the database correctly as

{"graph.dimensions":["CREATED_AT"],"graph.metrics":["expression","count","avg"]}

However, during load, we do some normalization on the loaded data where expression is a reserved term and the loading code does something to it.

metabase/src/metabase/models/interface.clj

Lines 107 to 128 in bf2ba1f

	(defn- normalize-visualization-settings [viz-settings]
	;; frontend uses JSON-serialized versions of MBQL clauses as keys in `:column_settings`; we need to normalize them
	;; to modern MBQL clauses so things work correctly
	(letfn [(normalize-column-settings-key [k]
	(some-> k u/qualified-name json/parse-string normalize/normalize json/generate-string))
	(normalize-column-settings [column-settings]
	(into {} (for [[k v] column-settings]
	[(normalize-column-settings-key k) (walk/keywordize-keys v)])))
	(mbql-field-clause? [form]
	(and (vector? form)
	(#{"field-id" "fk->" "datetime-field" "joined-field" "binning-strategy" "field"
	"aggregation" "expression"}
	(first form))))
	(normalize-mbql-clauses [form]
	(walk/postwalk
	(fn [form]
	(cond-> form
	(mbql-field-clause? form) normalize/normalize))
	form))]
	(cond-> (walk/keywordize-keys (dissoc viz-settings "column_settings"))
	(get viz-settings "column_settings") (assoc :column_settings (normalize-column-settings (get viz-settings "column_settings")))
	true normalize-mbql-clauses)))

#16259 changed the saved data to use the name as a reference instead of expression which fixes the issue (unless you happen to name your CE expression)

{"graph.dimensions":["CREATED_AT"],"graph.metrics":["Discount %","count","avg"]}

Note that the reproduction will still fail because it hard-coded the query to not have a name while in fact if you create it via the UI it will now have a name.

This also means that existing CEs will still fail, so the CE should be deleted and re-created.

[flamber]

@pawit-metabase Great findings. I don't think we can close this until we create a fix for existing questions.