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

Cannot use "Zoom in" action drill-through when grouped by Custom Column #13289

Closed
flamber opened this issue Sep 22, 2020 · 5 comments · Fixed by #16664
Closed

Cannot use "Zoom in" action drill-through when grouped by Custom Column #13289

flamber opened this issue Sep 22, 2020 · 5 comments · Fixed by #16664
Assignees
Labels
.Frontend Priority:P2 Average run of the mill bug .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone

Comments

@flamber
Copy link
Contributor

flamber commented Sep 22, 2020

Describe the bug
When a question is grouped by a Custom Column, then it's not possible to use "Zoom in" drill-through action.

To Reproduce

  1. Custom question > Sample Dataset > Orders
  2. Custom Column 1 + 1 as "TestColumn"
  3. Summarize Count by "TestColumn" and CreatedAt:Month
    image
  4. Visualize and click any of the line data points and click "Zoom in" - fails with
    Value does not match schema: {:query {:breakout [(named [nil (named (named (not (some-matching-condition? a-clojure.lang.PersistentVector)) "Must be a valid instance of one of these clauses: :field-id, :field-literal, :fk->, :joined-field") "field") nil] "Must be a valid instance of one of these clauses: :field-id, :field-literal, :joined-field, :fk->, :datetime-field, :expression, :binning-strategy")]}}
Full stacktrace
09-22 11:16:39 ERROR middleware.catch-exceptions :: Error processing query: null
{:database_id 4,
 :started_at #t "2020-09-22T11:16:39.715090+02:00[Europe/Copenhagen]",
 :error_type :invalid-query,
 :json_query
 {:type "query",
  :query
  {:source-table 11,
   :expressions {:TestColumn ["+" 1 1]},
   :aggregation [["count"]],
   :filter ["and" ["=" ["expression" "TestColumn"] 2] ["=" ["datetime-field" ["field-id" 86] "month"] "2018-01-01T00:00:00+01:00"]],
   :breakout [["datetime-field" ["expression" "TestColumn"] "week"]]},
  :database 4,
  :parameters [],
  :middleware {:js-int-to-string? true, :add-default-userland-constraints? true}},
 :native nil,
 :status :failed,
 :class clojure.lang.ExceptionInfo,
 :stacktrace
 ["--> util.schema$schema_core_validator$fn__21086.invoke(schema.clj:26)"
  "query_processor.middleware.validate$validate_query$fn__47230.invoke(validate.clj:9)"
  "query_processor.middleware.normalize_query$normalize$fn__45648.invoke(normalize_query.clj:22)"
  "query_processor.middleware.add_rows_truncated$add_rows_truncated$fn__39280.invoke(add_rows_truncated.clj:36)"
  "query_processor.middleware.results_metadata$record_and_return_metadata_BANG_$fn__47197.invoke(results_metadata.clj:147)"
  "query_processor.middleware.constraints$add_default_userland_constraints$fn__44927.invoke(constraints.clj:42)"
  "query_processor.middleware.process_userland_query$process_userland_query$fn__46389.invoke(process_userland_query.clj:136)"
  "query_processor.middleware.catch_exceptions$catch_exceptions$fn__44870.invoke(catch_exceptions.clj:174)"
  "query_processor.reducible$async_qp$qp_STAR___38074$thunk__38075.invoke(reducible.clj:101)"
  "query_processor.reducible$async_qp$qp_STAR___38074.invoke(reducible.clj:107)"
  "query_processor.reducible$sync_qp$qp_STAR___38083$fn__38086.invoke(reducible.clj:133)"
  "query_processor.reducible$sync_qp$qp_STAR___38083.invoke(reducible.clj:132)"
  "query_processor$process_userland_query.invokeStatic(query_processor.clj:215)"
  "query_processor$process_userland_query.doInvoke(query_processor.clj:211)"
  "query_processor$fn__47372$process_query_and_save_execution_BANG___47381$fn__47384.invoke(query_processor.clj:227)"
  "query_processor$fn__47372$process_query_and_save_execution_BANG___47381.invoke(query_processor.clj:219)"
  "query_processor$fn__47416$process_query_and_save_with_max_results_constraints_BANG___47425$fn__47428.invoke(query_processor.clj:239)"
  "query_processor$fn__47416$process_query_and_save_with_max_results_constraints_BANG___47425.invoke(query_processor.clj:232)"
  "api.dataset$fn__50707$fn__50710.invoke(dataset.clj:55)"
  "query_processor.streaming$streaming_response_STAR_$fn__35496$fn__35497.invoke(streaming.clj:73)"
  "query_processor.streaming$streaming_response_STAR_$fn__35496.invoke(streaming.clj:72)"
  "async.streaming_response$do_f_STAR_.invokeStatic(streaming_response.clj:66)"
  "async.streaming_response$do_f_STAR_.invoke(streaming_response.clj:64)"
  "async.streaming_response$do_f_async$fn__23282.invoke(streaming_response.clj:85)"],
 :context :ad-hoc,
 :error
 "Value does not match schema: {:query {:breakout [(named [nil (named (named (not (some-matching-condition? a-clojure.lang.PersistentVector)) \"Must be a valid instance of one of these clauses: :field-id, :field-literal, :fk->, :joined-field\") \"field\") nil] \"Must be a valid instance of one of these clauses: :field-id, :field-literal, :joined-field, :fk->, :datetime-field, :expression, :binning-strategy\")]}}",
 :row_count 0,
 :running_time 0,
 :preprocessed nil,
 :ex-data
 {:type :schema.core/error,
  :value
  {:type :query,
   :query
   {:source-table 11,
    :expressions {:TestColumn [:+ 1 1]},
    :aggregation [[:count]],
    :filter [:and [:= [:expression "TestColumn"] 2] [:= [:datetime-field [:field-id 86] :month] "2018-01-01T00:00:00+01:00"]],
    :breakout [[:datetime-field [:expression "TestColumn"] :week]]},
   :database 4,
   :middleware {:js-int-to-string? true, :add-default-userland-constraints? true},
   :info
   {:executed-by 1,
    :context :ad-hoc,
    :nested? false,
    :query-hash [20, -75, 21, 70, -78, 70, -112, -113, 54, -25, -77, 37, -120, -52, 1, -43, 98, 61, -124, 99, -22, -88, 9, 36, -48, -78, -18, 34, -98, 81, -113, 34]},
   :constraints {:max-results 10000, :max-results-bare-rows 2000}},
  :error
  {:query
   {:breakout
    [(named [nil (named (named (not (some-matching-condition? a-clojure.lang.PersistentVector)) "Must be a valid instance of one of these clauses: :field-id, :field-literal, :fk->, :joined-field") "field") nil] "Must be a valid instance of one of these clauses: :field-id, :field-literal, :joined-field, :fk->, :datetime-field, :expression, :binning-strategy")]}}},
 :data {:rows [], :cols []}}

09-22 11:16:39 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 118.0 ms (3 DB calls) App DB connections: 0/10 Jetty threads: 3/50 (3 idle, 0 queued) (82 total active threads) Queries in flight: 0 (0 queued)

Information about your Metabase Installation:
Metabase 0.34.2, 0.35.3 and 0.36.6

Additional context
The error is partially similar to #12938, but I don't think they are related much.

@flamber flamber added Type:Bug Product defects Priority:P2 Average run of the mill bug Querying/Processor Querying/MBQL labels Sep 22, 2020
@nemanjaglumac nemanjaglumac added this to Issues to Test in Cypress Testing Sep 22, 2020
@nemanjaglumac nemanjaglumac moved this from Issues to Test to In progress in Cypress Testing Sep 23, 2020
nemanjaglumac added a commit that referenced this issue Sep 23, 2020
@nemanjaglumac nemanjaglumac moved this from In progress to Unfixed Issue (but cypress repro has been made) in Cypress Testing Sep 23, 2020
nemanjaglumac added a commit that referenced this issue Sep 23, 2020
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Oct 1, 2020
camsaul added a commit that referenced this issue Oct 5, 2020
* Fix footer on release-0.36.x (#13287)

* Repro for issue #13241 (#13252)

* Add custom "hack" for cypress' broken `.type()` functionality
(This is most likely the cause of many flakes we're experiencing in CI.
More info can be found here: cypress-io/cypress#5480)

* Add repro for #13241 (already fixed)
* Split and distinguish repro test from functionality validation

* Add repro for #12762

Enable when underlying issue is fixed and the test should pass.

* Add repro for #13289 (#13300)

Co-authored-by: flamber <1447303+flamber@users.noreply.github.com>
Co-authored-by: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
@camsaul
Copy link
Member

camsaul commented Jun 7, 2021

I'm guessing the MBQL schema is just not allowing :expression in some place where we probably should allow it

@camsaul
Copy link
Member

camsaul commented Jun 16, 2021

The "Zoom In" option is not even popping up on the frontend anymore, so I'm not able to even reproduce the backend error. It fails with

drilldown.js:414 Uncaught TypeError: Cannot read property 'fields' of undefined
    at nextBreakouts (drilldown.js:414)
    at drillDownForDimensions (drilldown.js:434)
    at webpackJsonp../modes/components/drill/ZoomDrill.jsx.exports.default (ZoomDrill.jsx:19)
    at Mode.js:44
    at Array.flatMap (<anonymous>)
    at Mode.actionsForClick (Mode.js:43)
    at Visualization.getClickActions (Visualization.jsx:336)
    at Visualization._this.handleVisualizationClick (Visualization.jsx:326)
    at SVGRectElement.onClick (apply_tooltips.js:287)
    at SVGRectElement.__onmousedown (d3.js:1120)

I checked and table is undefined here:

const tableDimensions = table.fields.map(field =>

Going to recategorize this as a FE bug of unknown difficulty.

@daltojohnso
Copy link
Contributor

I was able to fix this partially by making some logic exclusive to FieldDimensions. PR here: #16664

A lot of logic in drilldown.js assumes you have a FieldDimension. For example, we try to grab tableId off a dimension.field() call and ExpressionDimensions don't have that property. Wasn't sure how to add it. Skipping all this logic for ExpressionDimensions fixes drilldown but the automatic dashboards are broken with an error:

"No method in multimethod 'automagic-analysis' for dispatch value: null"

@camsaul -- do you know if the automatic dashboards assume you have a FieldDimension? I don't know much about this part of the codebase.

@daltojohnso
Copy link
Contributor

Opened a new bug as a result of this fix -- #16680

This was referenced May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Frontend Priority:P2 Average run of the mill bug .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
None yet
5 participants