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

UI prevents adding 2 parameters to Percentile() function #15714

Closed
chasleslr opened this issue Apr 21, 2021 · 2 comments · Fixed by #15724
Closed

UI prevents adding 2 parameters to Percentile() function #15714

chasleslr opened this issue Apr 21, 2021 · 2 comments · Fixed by #15724
Assignees
Labels
Priority:P2 Average run of the mill bug Querying/Notebook Items specific to the Custom/Notebook query builder .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone

Comments

@chasleslr
Copy link

Describe the bug
When attempting to add a Custom Expression with the Percentile() function, the docstring indicates that 2 parameters are expected, but the validation returns an error when adding 2 parameters with the following error:

Function Percentile expects 1 argument

Logs
These are the logs for when incorrectly attempting to run the Percentile() function with only 1 argument. The validation error appears to be entirely UI based and so I could not find any logs for it.

[679465e5-6615-4bcb-a3ed-b309bb259bdf] 2021-04-21T11:58:54-04:00 ERROR metabase.query-processor.middleware.catch-exceptions Error processing query: null
{:database_id 3,
 :started_at #t "2021-04-21T15:58:53.842786Z[GMT]",
 :error_type :invalid-query,
 :json_query
 {:database 3,
  :query
  {:source-table 835, :aggregation [["aggregation-options" ["percentile" ["field" 10461 nil]] {:display-name "95%"}]]},
  :type "query",
  :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__16373.invoke(schema.clj:29)"
  "query_processor.middleware.validate$validate_query$fn__49352.invoke(validate.clj:9)"
  "query_processor.middleware.normalize_query$normalize$fn__47233.invoke(normalize_query.clj:22)"
  "query_processor.middleware.add_rows_truncated$add_rows_truncated$fn__44627.invoke(add_rows_truncated.clj:35)"
  "query_processor.middleware.results_metadata$record_and_return_metadata_BANG_$fn__49283.invoke(results_metadata.clj:147)"
  "query_processor.middleware.constraints$add_default_userland_constraints$fn__46478.invoke(constraints.clj:42)"
  "query_processor.middleware.process_userland_query$process_userland_query$fn__48395.invoke(process_userland_query.clj:135)"
  "query_processor.middleware.catch_exceptions$catch_exceptions$fn__46418.invoke(catch_exceptions.clj:173)"
  "query_processor.reducible$async_qp$qp_STAR___37956$thunk__37957.invoke(reducible.clj:103)"
  "query_processor.reducible$async_qp$qp_STAR___37956.invoke(reducible.clj:109)"
  "query_processor.reducible$sync_qp$qp_STAR___37965$fn__37968.invoke(reducible.clj:135)"
  "query_processor.reducible$sync_qp$qp_STAR___37965.invoke(reducible.clj:134)"
  "query_processor$process_userland_query.invokeStatic(query_processor.clj:239)"
  "query_processor$process_userland_query.doInvoke(query_processor.clj:235)"
  "query_processor$fn__49442$process_query_and_save_execution_BANG___49451$fn__49454.invoke(query_processor.clj:251)"
  "query_processor$fn__49442$process_query_and_save_execution_BANG___49451.invoke(query_processor.clj:243)"
  "query_processor$fn__49486$process_query_and_save_with_max_results_constraints_BANG___49495$fn__49498.invoke(query_processor.clj:263)"
  "query_processor$fn__49486$process_query_and_save_with_max_results_constraints_BANG___49495.invoke(query_processor.clj:256)"
  "api.dataset$run_query_async$fn__55699.invoke(dataset.clj:56)"
  "query_processor.streaming$streaming_response_STAR_$fn__55678$fn__55679.invoke(streaming.clj:72)"
  "query_processor.streaming$streaming_response_STAR_$fn__55678.invoke(streaming.clj:71)"
  "async.streaming_response$do_f_STAR_.invokeStatic(streaming_response.clj:65)"
  "async.streaming_response$do_f_STAR_.invoke(streaming_response.clj:63)"
  "async.streaming_response$do_f_async$fn__16071.invoke(streaming_response.clj:84)"],
 :context :ad-hoc,
 :error
 "Value does not match schema: {:query {:aggregation [[nil (named (named [nil nil (named (named (not (some-matching-condition? nil)) \"Must be a valid instance of one of these clauses: :expression, :field\") \"percentile\")] \"Must be a valid instance of one of these clauses: :count, :avg, :cum-count, :cum-sum, :distinct, :stddev, :sum, :min, :max, :metric, :share, :count-where, :sum-where, :case, :median, :percentile, :var\") \"aggregation\") nil]]}}",
 :row_count 0,
 :running_time 0,
 :preprocessed nil,
 :ex-data
 {:type :schema.core/error,
  :value
  {:database 3,
   :query
   {:source-table 835,
    :aggregation [[:aggregation-options [:percentile [:field 10461 nil] nil] {:display-name "95%"}]]},
   :type :query,
   :middleware {:js-int-to-string? true, :add-default-userland-constraints? true},
   :info
   {:executed-by 1,
    :context :ad-hoc,
    :nested? false,
    :query-hash
    [-114, 98, -95, 94, -45, -59, -110, -112, -10, 81, -24, -41, 92, -104, 116, 114, -52, 16, -54, 89, -121, 36, -121,
     -65, -29, -4, 27, 10, 57, -62, 46, 60]},
   :constraints {:max-results 10000, :max-results-bare-rows 2000}},
  :error
  {:query
   {:aggregation
    [[nil
      (named (named [nil nil (named (named (not (some-matching-condition? nil)) "Must be a valid instance of one of these clauses: :expression, :field") "percentile")] "Must be a valid instance of one of these clauses: :count, :avg, :cum-count, :cum-sum, :distinct, :stddev, :sum, :min, :max, :metric, :share, :count-where, :sum-where, :case, :median, :percentile, :var") "aggregation")
      nil]]}}},
 :data {:rows [], :cols []}}

To Reproduce
Steps to reproduce the behavior:

  1. Create a new Question
  2. Create a new Custom Expression using the Percentile() function
  3. Attempt to add a column, and a percentile-value (as shown in the docstring)
  4. See error: Function Percentile expects 1 argument

Expected behavior
I should be able to add 2 parameters to the Percentile() function.

Screenshots
Screen Shot 2021-04-21 at 11 55 40 AM

Adding 1 parameter correctly validates in the UI, however, it returns an error when attempting to run:
Screen Shot 2021-04-21 at 11 57 12 AM
Screen Shot 2021-04-21 at 11 57 24 AM

Information about your Metabase Installation:

You can get this information by going to Admin -> Troubleshooting.

  • Your browser and the version: Chrome/89.0.4389.128
  • Your operating system: Mac OS X 10.15.7
  • Your databases: Snowflake
  • Metabase version: v0.39.0
  • Metabase hosting environment: Elastic Beanstalk
  • Metabase internal database: Postgres

Severity
This is completely preventing the use of the Percentile function.

@flamber flamber added .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Priority:P2 Average run of the mill bug Querying/Notebook Items specific to the Custom/Notebook query builder and removed .Needs Triage labels Apr 21, 2021
@flamber
Copy link
Contributor

flamber commented Apr 21, 2021

Works on 0.38.4, but not on 0.39.0.1 - must have happened during an update to the Custom Expression formatter.

@ariya
Copy link
Contributor

ariya commented Apr 21, 2021

This is due the args mistake in frontend/src/metabase/lib/expressions/config.js (this data was ignored before 39, hence why the front-end did not complain before):

  percentile: {
    displayName: `Percentile`,
    type: "aggregation",
    args: ["number"],
    requiresFeature: "percentile-aggregations",
  },

I have a fix coming up soon.

ariya pushed a commit that referenced this issue Apr 21, 2021
ariya added a commit that referenced this issue Apr 21, 2021
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Apr 21, 2021
ariya added a commit that referenced this issue Apr 22, 2021
@flamber flamber linked a pull request Apr 22, 2021 that will close this issue
@flamber flamber added this to the 0.39.1 milestone Apr 22, 2021
@flamber flamber closed this as completed Apr 22, 2021
This was referenced Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:P2 Average run of the mill bug Querying/Notebook Items specific to the Custom/Notebook query builder .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
Development

Successfully merging a pull request may close this issue.

4 participants