Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Aug 04, 2022
  2. Aug 01, 2022
  3. Jul 28, 2022
  4. Jul 26, 2022
  5. Jul 25, 2022
    • Nemanja Glumac's avatar
      Unskip repro for #20809 (#24275) · 17bd41e8
      Nemanja Glumac authored
      Unverified
      17bd41e8
    • adam-james's avatar
      Add a check to PUT /user/:id to disallow name edits if an SSO user (#23752) · 3795b56c
      adam-james authored
      * Add a check to PUT /user/:id to disallow name edits if an SSO user
      
      * Clean up After SAML SSO tests
      
      The `:sso_source` key is set for the Rasta user in some SAML tests, but is expeted to be nil in subsequent tests, so
      we clean up in the SAML test ns.
      
      * Add a test to ensure SSO user names can't be changed via API
      
      * Missed a change I had made while adjusting tests
      
      * valid-name-update? take 1 name, to allow better error messages
      
      Let's the user know that first or last name is the cause of a problem, rather than just 'names'.
      
      * Remove unneeded thread macro
      
      * Use partial=
      
      * slight change to local fn for reusability
      Unverified
      3795b56c
    • Noah Moss's avatar
    • Anton Kulyk's avatar
      Remove unreachable QB editing mode feature code (#24261) · 2f177afc
      Anton Kulyk authored
      * Remove `isEditing` from QB state type
      
      * Remove `isEditing` from `updateQuestion` action
      
      * Remove `isEditing` from `setTemplateTag` action
      
      * Remove `isEditing` from QB state code
      Unverified
      2f177afc
    • Case Nelson's avatar
      [Actions] Add emitter and action tests (#24199) · e6aded3d
      Case Nelson authored
      * wip
      
      * Update kondo hook for with-temp*
      
      `with-temp*` allows for unpaired bindings if you just want the default
      model. This would break the `let` that the hook was binding if you had
      an unpaired temp model followed by others.
      
      * Adding tests for emitters and actions
      
      Test hydration of :emitters
      
      Test hydration of :action
      
      Test http emitter execution
      
      Test emitter crud
      
      Test failure cases around emitter execution
      
      Small consistency changes made to non-test code that shouldn't affect happy path FE code, largely 404 checking and keyword/string consistency.
      
      * Fix unused requires
      
      * Fix macro resolution
      Unverified
      e6aded3d
    • Noah Moss's avatar
    • dpsutton's avatar
      Ignore fields in joins when hoisting joined fields (#24167) · 0dc38ac3
      dpsutton authored
      * Ignore fields in joins when hoisting joined fields
      
      Need to understand two sides of mbql to understand this.
      
      ```clojure
      (defn nest-expressions
        "Pushes the `:source-table`/`:source-query`, `:expressions`, and `:joins` in the top-level of the query into a
        `:source-query` and updates `:expression` references and `:field` clauses with `:join-alias`es accordingly. See
        tests for examples. This is used by the SQL QP to make sure expressions happen in a subselect."
        ...)
      ```
      
      Make a query against the orders table joined to the people table and
      select order id, order total, and the person's email:
      
      ```sql
       SELECT "PUBLIC"."orders"."id"    AS "ID",
             "PUBLIC"."orders"."total" AS "TOTAL",
             "People - User"."email"   AS "People - User__EMAIL"
      FROM   "PUBLIC"."orders"
             LEFT JOIN "PUBLIC"."people" "People - User"
                    ON "PUBLIC"."orders"."user_id" = "People - User"."id"
      LIMIT  1048575
      ```
      
      Now add a custom column called adjective, which is 'expensive' when
      total > 100 else 'cheap'
      ```sql
       SELECT "source"."id"                   AS "ID",
             "source"."total"                AS "TOTAL",
             "source"."adjective"            AS "adjective",
             "source"."people - user__email" AS "People - User__EMAIL"
      FROM   (SELECT "PUBLIC"."orders"."id"         AS "ID",
                     "PUBLIC"."orders"."user_id"    AS "USER_ID",
                     "PUBLIC"."orders"."product_id" AS "PRODUCT_ID",
                     "PUBLIC"."orders"."subtotal"   AS "SUBTOTAL",
                     "PUBLIC"."orders"."tax"        AS "TAX",
                     "PUBLIC"."orders"."total"      AS "TOTAL",
                     "PUBLIC"."orders"."discount"   AS "DISCOUNT",
                     "PUBLIC"."orders"."created_at" AS "CREATED_AT",
                     "PUBLIC"."orders"."quantity"   AS "QUANTITY",
                     CASE
                       WHEN "PUBLIC"."orders"."total" > 50 THEN 'expensive'
                       ELSE 'cheap'
                     end                            AS "adjective",
                     "People - User"."email"        AS "People - User__EMAIL",
                     "People - User"."id"           AS "People - User__ID"
              FROM   "PUBLIC"."orders"
                     LEFT JOIN "PUBLIC"."people" "People - User"
                            ON "PUBLIC"."orders"."user_id" = "People - User"."id")
             "source"
      LIMIT  1048575
      ```
      
      We put the "work" in a nested query and then just update the outer
      select to select `source.total` instead of `orders.total`. But the way
      this figured out which joins to hoist up was a bit broken. It walked all
      over the inner query, finding fields it was interested in. However, it
      also got fields it shouldn't have, by descending into join information
      that should be opaque at this level.
      
      In the repro for this, we make a card Q1, selecting product_id and
      count, but with an implicit join to products and filtering where
      category = doohickey.
      
      ```clojure
      ;; card Q1
      {:type :query,
       :query {:source-table 4,     #_ reviews
               :filter [:= [:field 26 {:source-field 33}] "Doohickey"], #_ products.category
               :aggregation [[:count]],
               :breakout [[:field 33 nil]],  #_ reviews.product_id
               :limit 2},
       :database 1}
       ```
      
      A second question Q2 queries the Orders table and joins to Q1 on
      orders.product_id = q1.product_id, and adds a custom column `+ 1 1`.
      ```
      ;; card Q2, based on Q1
      {:source-table 2,
       :expressions {"CC" [:+ 1 1]},
       :fields [[:field 9 nil]
                [:field 3 nil]
                [:field 5 nil]
                [:field 6 nil]
                [:field 8 nil]
                [:field 7 nil]
                [:field 1 nil]
                [:field 4 {:temporal-unit :default}]
                [:field 2 nil]
                [:expression
                 "CC"
                 {:metabase.query-processor.util.add-alias-info/desired-alias "CC",
                  :metabase.query-processor.util.add-alias-info/position 9}]
                [:field 33 nil]
                [:field "count" {:base-type :type/BigInteger}]],
       :joins [{:alias "Question 4918",
                :strategy :left-join,
                :fields [[:field 33 nil]
                         [:field "count" {:base-type :type/BigInteger}]],
                :condition [:=
                            [:field 5 nil]
                            [:field 33 nil]],
                :source-card-id 4918,
                }]}
      ```
      
      and this should yield the sql:
      
      ```sql
      SELECT "source"."ID" AS "ID",
             "source"."PRODUCT_ID" AS "PRODUCT_ID",
             "source"."TOTAL" AS "TOTAL",
             "source"."CC" AS "CC",
             "source"."Question 4918__PRODUCT_ID" AS "Question 4918__PRODUCT_ID",
             "source"."Question 4918__count" AS "Question 4918__count"
      FROM (
        SELECT "PUBLIC"."ORDERS"."ID" AS "ID",
               "PUBLIC"."ORDERS"."USER_ID" AS "USER_ID",
               "PUBLIC"."ORDERS"."PRODUCT_ID" AS "PRODUCT_ID",
               "PUBLIC"."ORDERS"."SUBTOTAL" AS "SUBTOTAL",
               "PUBLIC"."ORDERS"."TAX" AS "TAX",
               "PUBLIC"."ORDERS"."TOTAL" AS "TOTAL",
               "PUBLIC"."ORDERS"."DISCOUNT" AS "DISCOUNT",
               "PUBLIC"."ORDERS"."CREATED_AT" AS "CREATED_AT",
               "PUBLIC"."ORDERS"."QUANTITY" AS "QUANTITY",
               (1 + 1) AS "CC",
               "Question 4918"."PRODUCT_ID" AS "Question 4918__PRODUCT_ID",
               "Question 4918"."count" AS "Question 4918__count"
        FROM "PUBLIC"."ORDERS"
        LEFT JOIN (
          SELECT "PUBLIC"."REVIEWS"."PRODUCT_ID" AS "PRODUCT_ID",
                 count(*) AS "count"
          FROM "PUBLIC"."REVIEWS"
          LEFT JOIN "PUBLIC"."PRODUCTS" "PRODUCTS__via__PRODUCT_ID"
            ON "PUBLIC"."REVIEWS"."PRODUCT_ID" = "PRODUCTS__via__PRODUCT_ID"."ID"
          WHERE "PRODUCTS__via__PRODUCT_ID"."CATEGORY" = 'Doohickey'
          GROUP BY "PUBLIC"."REVIEWS"."PRODUCT_ID"
          ORDER BY "PUBLIC"."REVIEWS"."PRODUCT_ID" ASC) "Question 4918"
        ON "PUBLIC"."ORDERS"."PRODUCT_ID" = "Question 4918"."PRODUCT_ID") "source"
      LIMIT 1048575
      ```
      
      But when it was looking through to see which fields to hoist top level,
      it was also finding `"PRODUCTS__via__PRODUCT_ID"."ID"` and
      `"PRODUCTS__via__PRODUCT_ID"."CATEGORY"` as fields it should hoist up
      because it searched the whole tree underneath it which includes join
      conditions and such.
      
      But of course that doesn't matter, the only thing is really analyzing
      what fields come out of a query, and those fields do not come out of the
      nested query
      
      ```sql
          SELECT "PUBLIC"."REVIEWS"."PRODUCT_ID" AS "PRODUCT_ID",
                 count(*) AS "count"
          FROM "PUBLIC"."REVIEWS"
          LEFT JOIN "PUBLIC"."PRODUCTS" "PRODUCTS__via__PRODUCT_ID"
            ON "PUBLIC"."REVIEWS"."PRODUCT_ID" = "PRODUCTS__via__PRODUCT_ID"."ID"
          WHERE "PRODUCTS__via__PRODUCT_ID"."CATEGORY" = 'Doohickey'
          GROUP BY "PUBLIC"."REVIEWS"."PRODUCT_ID"
          ORDER BY "PUBLIC"."REVIEWS"."PRODUCT_ID" ASC) "Question 4918"
      ```
      
      that only returns product_id and count.
      
      And this matches the error it was (helpfully) throwing:
      
      ```clojure
      investigation=> (qp/compile nested-query)
      Execution error (ExceptionInfo) at metabase.query-processor.util.add-alias-info/field-source-table-alias (add_alias_info.clj:182).
      Cannot determine the source table or query for Field clause
      [:field
       26 #_ products.category
       {:source-field 33, #_reviews.product_id
        :join-alias "PRODUCTS__via__PRODUCT_ID",
        :metabase.query-processor.util.add-alias-info/source-table "PRODUCTS__via__PRODUCT_ID",
        :metabase.query-processor.util.add-alias-info/source-alias "CATEGORY"}]
      ```
      
      And here's the query it was analyzing when determining which fields it
      needed to hoist (looking for ones with `:join-alias` (which is also in
      the nest_query_test.clj file):
      
      ```clojure
      ;; removed some cruft and extra field information for brevity
      {:source-table 2,
       :expressions  {"CC" [:+ 1 1]},
       :fields       [[:field 33 {:join-alias "Question 4918",}]
                      [:field "count" {:join-alias "Question 4918"}]]
       :joins        [{:alias           "Question 4918",
                       :strategy        :left-join,
                       :fields          [[:field 33 {:join-alias "Question 4918"}]
                                         [:field
                                          "count"
                                          {:join-alias "Question 4918"}]]
                       :condition       [:=
                                         [:field 5 nil]
                                         [:field 33 {:join-alias "Question 4918",}]],
                       :source-card-id  4918,
                       :source-query    {:source-table 4,
                                         ;; nested query has filter values with join-alias that should not
                                         ;; be selected
                                         :filter       [:=
                                                        [:field 26 {:join-alias "PRODUCTS__via__PRODUCT_ID"}]
                                                        [:value "Doohickey" {}]],
                                         :aggregation  [[:aggregation-options
                                                         [:count]
                                                         {:name "count"}]],
                                         :breakout     [[:field 33 nil]],
                                         :limit        2,
                                         :order-by     [[:asc
                                                         [:field 33 nil]]],
                                         ;; nested query has an implicit join with conditions that should
                                         ;; not be selected
                                         :joins        [{:alias        "PRODUCTS__via__PRODUCT_ID",
                                                         :strategy     :left-join,
                                                         :condition    [:=
                                                                        [:field 33 nil]
                                                                        [:field
                                                                         30
                                                                         {:join-alias "PRODUCTS__via__PRODUCT_ID"}]]
                                                         :source-table 1,
                                                         :fk-field-id  33}]},
                       :source-metadata [{:field_ref [:field 33 nil]}
                                         {:field_ref [:aggregation 0]}]}]}
      ```
      
      * Add round-trip test through qp
      
      This test is essentially the repro from the ticket
      https://github.com/metabase/metabase/issues/20809
      
      ```clojure
      debug-qp=> (to-mbql-shorthand (:dataset_query (metabase.models/Card 4919)))
      (mt/mbql-query
       orders
       {:joins [{:source-table "card__4918",
                 :alias "Question 4918",
                 :condition [:=
                             $product_id
                             [:field
                              %reviews.product_id
                              {:join-alias "Question 4918"}]],
                 :fields :all}],
        :expressions {"CC" [:+ 1 1]}})
      debug-qp=> (to-mbql-shorthand (:dataset_query (metabase.models/Card 4918)))
      (mt/mbql-query
       reviews
       {:breakout [$product_id],
        :aggregation [[:count]],
        :filter [:= $product_id->products.category "Doohickey"]})
      ```
      
      Thanks to @cam for providing such a lovely tool
      Unverified
      0dc38ac3
    • Braden Shepherdson's avatar
      Serdes v2: Add `--v2 true` flag to the `dump` and `load` commands (#24230) · 6d46ef12
      Braden Shepherdson authored
      * Serdes v2: Add `--v2 true` flag to the `dump` and `load` commands
      
      This is the last piece for end-to-end serialization and deserialization.
      The results should be similar to v1, though probably not identical.
      Unverified
      6d46ef12
    • Ryan Laurie's avatar
      show date empty and not empty (#24240) · a65774b1
      Ryan Laurie authored
      Unverified
      a65774b1
    • Alexander Polyankin's avatar
    • Gustavo Saiani's avatar
    • Natalie's avatar
      docs - update coalesce (#24241) · 83591847
      Natalie authored
      Unverified
      83591847
    • Anton Kulyk's avatar
      Convert to TypeScript (#24143) · 8d6bce2c
      Anton Kulyk authored
      Unverified
      8d6bce2c
    • GitStart's avatar
    • Anton Kulyk's avatar
      Remove unreachable "SQL preview mode" feature code from QB (#24257) · 01ca7cdd
      Anton Kulyk authored
      * Remove `isPreview` mentions in QB components
      
      * Remove preview mode usage in `runQuestionQuery`
      
      * Remove preview related components
      
      * Remove preview code from QB store
      
      * Remove `isPreview` from QB state type
      Unverified
      01ca7cdd
    • Dalton's avatar
      Add TagEditorParam unit tests (#24196) · 90d7cdd4
      Dalton authored
      * Add TagEditorParam unit tests
      
      * Add prop-types & export component
      Unverified
      90d7cdd4
    • Anton Kulyk's avatar
      Refactor QB `updateQuestion` action (#24142) · fe429905
      Anton Kulyk authored
      * Strip unnecessary comments
      
      * Rename `oldQuestion` > `currentQuestion`
      
      * Rename `mode` > `queryBuilderMode`
      
      * Extract `shouldTurnIntoAdHoc` condition
      
      * Break down pivot table section
      
      * Simplify template tag editor block
      
      * Minor style tweak
      
      * Remove moved helper
      
      * Remove export
      
      * Fix missing `dispatch`
      Unverified
      fe429905
    • Gustavo Saiani's avatar
    • Anton Kulyk's avatar
    • Anton Kulyk's avatar
      Move QB `updateQuestion` and shared utils to their own files (#24140) · f89b7109
      Anton Kulyk authored
      * Move shared utility to its own file
      
      * Move `loadMetadataForCard` to its own file
      
      * Move shared utility to its own file
      
      * Move `loadMetadataForCard` to its own file
      
      * Move `updateQuestion` action to its own file
      
      * Export extracted actions
      
      * Fix references to moved actions
      
      * Run prettier
      
      * Fix old question sidebar code usage
      Unverified
      f89b7109
  6. Jul 23, 2022
  7. Jul 22, 2022
Loading