Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Apr 03, 2023
    • dpsutton's avatar
      Manual backport persistent schema cache info table (#29741) · 03efe54d
      dpsutton authored
      * Create `cache_info` table in our persisted schemas
      
      ```sql
      -- postgres/redshift
      test-data=# select * from metabase_cache_424a9_379.cache_info ;
             key        |                value
      ------------------+--------------------------------------
       settings-version | 1
       created-at       | 2023-03-28
       instance-uuid    | 407e4ba8-2bab-470f-aeb5-9fc63fd18c4e
       instance-name    | Metabase Test
      (4 rows)
      ```
      
      ```sql
      --mysql
      mysql> select * from metabase_cache_424a9_435.cache_info;
      +------------------+--------------------------------------+
      | key              | value                                |
      +------------------+--------------------------------------+
      | settings-version | 1                                    |
      | created-at       | 2023-03-28                           |
      | instance-uuid    | 407e4ba8-2bab-470f-aeb5-9fc63fd18c4e |
      | instance-name    | Metabase Test                        |
      +------------------+--------------------------------------+
      ```
      
      our key values in v1:
      ```clojure
      (defn kv-table-values
        "Version 1 of the values to go in the key/value table `cache_info` table."
        []
        [{:key   "settings-version"
          :value "1"}
         {:key   "created-at"
          ;; "2023-03-28"
          :value (.format (LocalDate/now) DateTimeFormatter/ISO_LOCAL_DATE)}
         {:key   "instance-uuid"
          :value (public-settings/site-uuid)}
         {:key   "instance-name"
          :value (public-settings/site-name)}])
      ```
      
      This will enable us to delete cached schemas in shared databases (cloud
      versions like redshift) without risking wiping out a cached schema
      during an active test run.
      
      The code to delete the schemas will be something like,
      
      ```clojure
      (def spec (sql-jdbc.conn/connection-details->spec
                 :redshift
                 {:host "xxx.us-east-1.reshift.amazonaws.com"
                  :db "db"
                  :port 5439
                  :user "user"
                  :password "password"}))
      
      (let [days-prior 1 ;; probably 2 to handle crossing day time. Can also adjust to 6 hours, etc
            threshold (.minus (java.time.LocalDate/now)
                              days-prior
                              java.time.temporal.ChronoUnit/DAYS)]
        (doseq [schema (->> ["select nspname from pg_namespace where nspname like 'metabase_cache%'"]
                            (jdbc/query spec)
                            (map :nspname))]
          (let [[{created :value}] (jdbc/query spec (sql/format {:select [:value]
                                                                 :from [(keyword schema "cache_info")]
                                                                 :where [:= :key "created-at"]}
                                                                {:dialect :ansi}))]
            (when created
              (let [creation-day (java.time.LocalDate/parse created)]
                (when (.isBefore creation-day threshold)
                  (jdbc/execute! spec [(format "drop schema %s cascade" schema)])))))))
      ```
      
      or if we want to do it in a single query:
      
      ```clojure
      schemas=> (let [days-prior 1
                      threshold  (.minus (java.time.LocalDate/now)
                                         days-prior
                                         java.time.temporal.ChronoUnit/DAYS)]
                  (let [schemas (->> ["select nspname
                               from pg_namespace
                               where nspname like 'metabase_cache%'"]
                                     (jdbc/query spec)
                                     (map :nspname))]
                    (jdbc/with-db-connection [conn spec]
                      (jdbc/execute! conn [(format "set search_path= %s;" (str/join ", " schemas))])
                      (let [sql               (sql/format {:select [:schema :created-at]
                                                           :from   {:union-all
                                                                    (for [schema schemas]
                                                                      {:select [[[:inline schema] :schema]
                                                                                [{:select [:value]
                                                                                  :from   [(keyword schema "cache_info")]
                                                                                  :where  [:= :key [:inline "created-at"]]}
                                                                                 :created-at]]})}}
                                                          {:dialect :ansi})
                            old?              (fn [{create-str :created-at}]
                                                (let [created (java.time.LocalDate/parse create-str)]
                                                  (.isBefore created threshold)))
                            schemas-to-delete (->> (jdbc/query spec sql)
                                                   (filter old?)
                                                   (map :schema))]
                        schemas-to-delete))))
      ("metabase_cache_424a9_503") ;; when there are any, string join them and delete them as above
      ```
      
      * Use Instants so we can compare by hours instead of just days
      
      * require toucan as db
      03efe54d
    • Noah Moss's avatar
      backport 29738 (#29755) · e5aded4e
      Noah Moss authored
      e5aded4e
  2. Mar 24, 2023
    • metabase-bot[bot]'s avatar
      Separate date from datetime binning options (#29423) (#29504) · 7b132cad
      metabase-bot[bot] authored
      * Separate date from datetime binning options
      
      The BE sends over binning indexes for each field and a top level map for
      how to interpret these keys.
      
      ```javascript
      ❯ http get http://localhost:3000/api/table/10658/query_metadata
      
       Cookie:$SESSION -pb
      {
          "active": true,
          "caveats": null,
          "created_at": "2023-03-21T22:20:42.363169Z",
          "db": {
              "engine": "postgres",
              "features": [ "full-join", ... ],
              "id": 499,
              ...
          },
          "db_id": 499,
          "dimension_options": {
              "0": { "mbql": [ "field", null, { "temporal-unit": "day" } ],
                     "name": "Day",
                     "type": "type/Date"},
              "1": { "mbql": [ "field", null, { "temporal-unit": "week" } ],
                     "name": "Week",
                     "type": "type/Date" },
              "10": { "mbql": ["field", null, { "temporal-unit": "quarter-of-year"}],
                      "name": "Quarter of Year",
                      "type": "type/Date" },
              "11": { "mbql": [ "field", null, { "temporal-unit": "minute" } ],
                      "name": "Minute",
                      "type": "type/DateTime"},
              "12": { "mbql": [ "field", null, { "temporal-unit": "hour"}],
                      "name": "Hour",
                      "type": "type/DateTime"},
              ... // more options. They have a type, an id, and an mbql clause
          },
          "display_name": "Different Times",
          "entity_type": null,
          "fields": [{"name": "id",},
                     {"name": "dt",
                      "dimension_options": [
                          "11", "12", "13", "14", "15", "16", "17",
                          "18", "19", "20", "21", "22", "23", "24", "25"]
                      "database_type": "timestamp",
                      "effective_type": "type/DateTime",
                     },
                     {"name": "t",
                      "dimension_options": [ "26", "27", "28" ],
                      "effective_type": "type/Time",
                      "database_type": "time"
                     },
                     {"name": "d",
                      "database_type": "date",
                      "dimension_options": [ "0", "1", "2", "3", "4", "5",
                                             "6", "7", "8", "9", "10" ],
                      "effective_type": "type/Date",
              }
          ],
          "name": "different_times",
          ...
      }
      ```
      
      This PR adds some new types just for dates so that you don't get offered
      by group a date by hour or minute.
      
      This adds new indexes to this list, which makes us wary that these
      indexes have leaked out and cannot be changed. But that is not the
      case. This is essentially compression on the response. When the FE saves
      a query, it does not include the index but uses the mbql query to insert
      the proper temporal-unit:
      
      ```clojure
      ;; a "date" field choosing the quarter option
      {:type :query,
       :query {:source-table 5,
               :breakout [[:field 49 {:temporal-unit :quarter}]],
               :aggregation [[:count]]},
       :database 1}
      
      ;; the same query but binning by week
      {:database 1,
       :query {:source-table 5,
               :breakout [[:field 49 {:temporal-unit :week}]],
               :aggregation [[:count]]},
       :type :query}
      ```
      
      So the index is relative to just the editing session of the query, and
      the indexes do not have to be stable when we add new ones.
      
      * athena and mongo (others?) don't have a date type
      
      the test expects a date field to have date binning options. previous, we
      made no distinction for binning options for date vs datetime fields. Now
      we do and have to manage expectations in the tests.
      
      * mongo returns type/Instant
      
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
  3. Mar 23, 2023
  4. Mar 22, 2023
  5. Mar 21, 2023
  6. Mar 20, 2023
    • Cal Herries's avatar
      Optimize DB calls of dashboard card updates some more - 2 (#29252) (#29320) · f951db30
      Cal Herries authored
      * Transform card data into the same format as that which comes out of the DB
      
      * Use `from-json` instead of toucan transformations
      
      * Remove unused require
      
      * Update docstring and rename to from-parsed-json
      
      * Oops, fixed
      
      * Remove now
      
      * Add docstring
      
      * Fix order of docstring
      
      * Don't update created_at and updated_at
      f951db30
  7. Mar 17, 2023
  8. Mar 16, 2023
    • metabase-bot[bot]'s avatar
      Ensure we use the ssh tunnel on action execution (#29228) (#29300) · 5b795e63
      metabase-bot[bot] authored
      
      * Ensure we use the ssh tunnel on action execution
      
      I'm gonna see if we have an existing way to run tests against ssh dbs. I
      see there are some tests that use a faking ssh server. Not sure if those
      are useful or not. But otherwise ssh stuff needs out of process help to
      accomplish so not sure if it's easily feasible right now
      
      * introduce new `with-unpooled-connection-spec` macro
      
      it is identical to the `with-connection-spec-for-testing-connection`
      macro but has a better name. In the future they could diverge as testing
      requirements and action requirements change. But we want that
      _implementation_ (which correctly uses tunnels), but we do not want a
      "testing connection" slug in the usage.
      
      In the future (or now if we prefer a different name) we can rename it,
      find the usages that are not testing vs the ones that are, etc.
      
      * tests for actions over ssh
      
      * unused require of mb.u.ssh
      
      had brought it in but then stuffed all of this inside of
      `with-unpooled-connection-spec` so no need to worry about ssh, closing
      it, ensuring the port is there, etc.
      
      * Don't test h2 with ssh tunnel
      
      details lack a host and it npe's in the ssh tunnel stuff. No big loss as
      ssh on h2 is not supported (due to this error, and probably many more)
      
      * Switch to toucan2 for db access in tests
      
      * Use connection pool for custom actions connection
      
      Was originally using `jdbc/get-connection` from the details of the
      db. This was a one-off non-pooled connection that also did not reuse ssh
      tunnels. I thought the one-off connection was important since it is not
      read only, but that's not the case. So we can reuse the exact same
      mechanism that the implicit actions are using with
      `metabase.driver.sql-jdbc.actions/with-jdbc-transaction`.
      
      * cyclic dependency
      
      almost moved it into execute, but that gives a very inviting
      `sql-jdbc.execute/with-jdbc-transaction`. Welcoming even. But
      `sql-jdbc.actions/with-jdbc-transaction` lets you know its only for
      transactions.
      
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      5b795e63
  9. Mar 15, 2023
  10. Mar 14, 2023
  11. Mar 13, 2023
    • metabase-bot[bot]'s avatar
      Fix flaky test and test for a new case (#28757) (#28989) · e243c36c
      metabase-bot[bot] authored
      
      * Fix flaky test and test for a new case
      
      flaky test was annoying. It would timeout after 45 seconds. And
      annoyingly, it times out on the _client_, not the server. So we don't
      get the proper error message we expect.
      
      And what's happening is that the backend just hooks up the pipe from
      it's request to the response. And if no bytes ever come across, we just
      twiddle our thumbs until the http request in the test gives up.
      
      The only reliable way i could think of to fix this is to spin up a
      webserver that just accepts and then ignores requests (which is what
      github did to us in the flaky test).
      
      Had to bump the ring adapter to get the newly exposed `:async-timeout
      60000` parameter. By default the webserver in the test has a timeout of
      30 seconds, which is shorter than the 45 second timeout for the test's
      request to the geojson endpoint.
      
      * add comment
      
      * clean up test server logic
      
      ---------
      
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      Co-authored-by: default avatarNemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
      e243c36c
    • metabase-bot[bot]'s avatar
      Cannot use filter relative interval on breakout fields (#28799) (#28930) · a43fe5a7
      metabase-bot[bot] authored
      
      Fixes #25378
      
      When using a relative filter with a starting from n units ago on an
      aggregation with the same units, we were incorrectly assuming that we
      would be working with a field ref at the top level. However, in this
      scenario the field is nested such as `[:+ [:field $created_at
      {:temporal-unit :month}] [:interval 1 :month]]`. This change makes sure
      that we are altering the field-ref.
      
      Co-authored-by: default avatarCase Nelson <case@metabase.com>
      a43fe5a7
    • metabase-bot[bot]'s avatar
      Handle models with joins (#29014) (#29125) · 50d27b32
      metabase-bot[bot] authored
      
      * Handle models with joins
      
      We had been using the metadata from the query to get column names, and
      then `select (column-names) from persisted-table`.
      
      But joins affect how columns are emitted.
      
      For this tests, using orders join to products, selecting order.total and
      product.category. And the metadata from running this query uses the
      normal names `:name "total"`, and `:name "category"`. _BUT_ the emitted
      sql does not use these names:
      
      ```sql
      SELECT "public"."orders"."total" AS "total",
             "products"."category" AS "products__category" -- <--------
      FROM "public"."orders"
      LEFT JOIN "public"."products" AS "products"
             ON "public"."orders"."product_id" = "products"."id"
      ```
      
      When we persist, we do `create table as <(qp/compile query)> so we were
      creating a column named `products__category` _NOT_ "category". But later
      on we would select "category" and this would blow up.
      
      ```sql
      select "total", "category" -- <- category doesn't exist in this table
      from "metabase_cache_424a9_8"."model_1105_txbhmkrnoy"
      ```
      
      Now a query of `{:source-table "card__<model-id>"}' emits the following
      query when the persisted cache is substituted:
      
      ```sql
      SELECT "source"."total" AS "total",
             "source"."products__category" AS "products__category"
      FROM (select * from "metabase_cache_424a9_8"."model_1152_lecqfzcjke") AS
      "source"
      LIMIT 1048575
      ```
      
      * Test and lint fixes
      
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      50d27b32
    • metabase-bot[bot]'s avatar
  12. Mar 12, 2023
    • metabase-bot[bot]'s avatar
      Improving axes grouping/splitting in static viz. (#28295) (#29148) · f21c7c21
      metabase-bot[bot] authored
      
      * WIP for better axes grouping in static viz.
      
      Double x-axis and single x-axis are different, but both can have situations where grouping might occur, so the
      implementation is a tad confusing. I'm trying to build test cases that represent both scenarios reasonably, and build
      an implementation that's not too confusing.
      
      * Fix reflection warnings in render test utils
      
      * Overlap calcs don't fail with BigDecimal inputs now.
      
      * Continuing to clean up overlaps calcs and impl for single/double in progress
      
      * WIP version that works for single-x multi-series and double-x multi
      
      There's a lot to think about here...
      
      But basically, handle rows of shape:
      
      double-x:
      
      [[x1 x2] [y]]
      
      and single-x:
      
      [[x] [y1 y2 y3]]
      
      * Single and Multi X axis now use same axes grouping strategy
      
      * Cover some overlap scoring cases.
      
      This may need improving in future still, but the overlap calculation is at least used for both single-x and multi-x
      axis scenarios now, which is a definitie improvement.
      
      * Missed these unused bindings in test let body
      
      * overlap calc won't fail if we pass in `[nil nil]`
      
      * Clean up grouping axes implementation addressing review feedback.
      
      - overlap and nearness split into two functions
      - add the fn 'axis-group-score' to call overlap/nearness appropriately, and handle condition checks (eg. nil vals)
      - removed old group-axes and now unnecessasry default-y-axis-pos fn -> handled by group-axes-at-once fn
      - axis group threshold not passed as arg anymore
      
      * Fixed some alignment mistakes
      
      Co-authored-by: default avataradam-james <21064735+adam-james-v@users.noreply.github.com>
      f21c7c21
  13. Mar 10, 2023
  14. Mar 08, 2023
    • Noah Moss's avatar
      Manual backport of #28961 to 46 release branch (#29007) · 54700412
      Noah Moss authored
      * Bind *card-id* when executing a dashboard subscription to fix perm checks (#28961)
      
      * bind *card-id* when executing a dashboard subscription so that perm checks work properly
      
      * refactor & address test comment
      
      * fix spacing
      
      * unskip repro that this also fixes
      
      * fix lint
      54700412
  15. Mar 02, 2023
  16. Mar 01, 2023
    • metabase-bot[bot]'s avatar
      Handle action db and model db different (#28616) (#28758) · 228d9050
      metabase-bot[bot] authored
      
      * Ensure query action's db matches model id
      
      This is only a half-way measure. Needs some FE loving as well.
      
      The +New -> Action workflow lets you create an action. This starts with
      the user choosing the database, writing the action. On save it prompts
      you to choose a model it should live under. But this model's db and the
      db chosen originally are not checked in the FE.
      
      Ideally we only show models on that existing db, but it kinda seems
      weird to have an action do things to a random table and it live under a
      model that just needs to be on the db.
      
      * reword error message
      
      
      
      * Check both databases for enabled actions when they disagree
      
      a custom action can have its own query which brings its own
      database. When creating a new custom action from the models detail page,
      these go hand in hand. When using the workflow from the "+New" button,
      the model is chosen at the last step, well after the action's db and
      query is set. And there's nothing holding these two together.
      
      Asked if this was a bug and was told no. But we want to make sure we
      check both for actions enabled.
      
      * fix lint
      
      ---------
      
      Co-authored-by: default avatardpsutton <dan@dpsutton.com>
      Co-authored-by: default avatarMaz Ameli <maz@metabase.com>
      228d9050
    • metabase-bot[bot]'s avatar
  17. Feb 28, 2023
    • dpsutton's avatar
      Delay substituting persisted native query until after analysis (#28739) · c4050fee
      dpsutton authored
      * Delay substituting persisted native query until after analysis
      
      We were substituting persisted queries into the query at too early a
      stage.
      
      Card 729 is a persisted model of `select * from categories`
      
      ```clojure
      {:type :query,
       :query {:source-table "card__729",
               :expressions {:adjective ["case"
                                         [[[">" ["field" 100 nil] 30]
                                           "expensive"]
                                          [[">" ["field" 100 nil] 20]
                                           "not too bad"]]
                                         {:default "its ok"}]},
               :aggregation [["count"]],
               :breakout [["expression" "adjective" nil]
                          ["field" 101 nil]]},
       :database 3,
       :parameters []}
      ```
      
      This would immediately get turned into an equivalent
      
      ```clojure
      {:type :query,
       :query {:source-table 12
               :persisted-info/native "select \"id\", \"ean\", \"title\", \"category\", \"vendor\", \"price\", \"rating\", \"created_at\" from \"metabase_cache_424a9_3\".\"model_729_products_r\""
               :expressions {:adjective ["case"
                                         [[[">" ["field" 100 nil] 30]
                                           "expensive"]
                                          [[">" ["field" 100 nil] 20]
                                           "not too bad"]]
                                         {:default "its ok"}]},
               :aggregation [["count"]],
               :breakout [["expression" "adjective" nil]
                          ["field" 101 nil]]},
       :database 3,
       :parameters []}
      ```
      
      And the obvious failure here is the use of `[:field 100 nil]` (price)
      against an inner query of `select ... price ... from cache_table`. This
      could break many queries and caused a flood of
      
      ```
      WARN middleware.fix-bad-references :: Bad :field clause [:field 100 #:metabase.query-processor.util.add-alias-info{:source-table :metabase.query-processor.util.add-alias-info/source, :source-alias "price"}] for field "products.price" at [:expressions "adjective" :case :>]: clause should have a :join-alias. Unable to infer an appropriate join. Query may not work as expected.
      WARN middleware.fix-bad-references :: Bad :field clause [:field 100 #:metabase.query-processor.util.add-alias-info{:source-table :metabase.query-processor.util.add-alias-info/source, :source-alias "price"}] for field "products.price" at [:expressions "adjective" :case :>]: clause should have a :join-alias. Unable to infer an appropriate join. Query may not work as expected.
      ```
      
      * forgot to commit new test file
      
      * Handle persisted before native
      
      * Add test for native models
      
      - have to populate the metadata. You can do this by running the query,
      but there are some requirements around the `:info` object to actually
      save it. So just take the more mechanical route
      - `wait-for-result` returns a time out value, but it's namespaced
      autoresolved to a namespace inside of the metabase.test forest and not
      mt itself. So easy to mess up and watch for the wrong keyword. Want an
      arity so i can check my own timeout value and throw a helpful error
      message. I checked around and noone seems to care. So not great, but at
      least people aren't checking `::mt/timed-out` when it should be
      `::tu.async/timed-out` or
      `:metabase.test.util.async/timed-out`. Possibly best scenario is we
      remove the autoresolved bit and have it return `:async/timed-out` or
      something stable.
      
      * properly compile the native query
      
      * bump timeout to set redshift metadata
      c4050fee
    • john-metabase's avatar
      Serdes v2 clean up collections option handling (#28745) · 0761928a
      john-metabase authored
      Cleans/fixes serdes v2 export :collections handling in cmd and API
      0761928a
    • john-metabase's avatar
Loading