Remove exploded json fields in implicit actions (#28558)
* Remove json fields from action parameters ```sql json-test=# select * from json_table; json_val | description ------------------+--------------- {"a": 1, "b": 2} | a description (1 row) ``` This ends up in our db as: ```clojure ({:database_type "json", :name "json_val", :effective_type :type/Structured, :nfc_path nil, :id 73, :display_name "Json Val", :database_required true} {:database_type "bigint", :name "json_val → a", :effective_type :type/Integer, :nfc_path ("json_val" "a"), :id 72, :display_name "Json Val → A", :database_required false} {:database_type "bigint", :name "json_val → b", :effective_type :type/Integer, :nfc_path ("json_val" "b"), :id 74, :display_name "Json Val → B", :database_required false} {:database_type "text", :name "description", :effective_type :type/Text, :nfc_path nil, :id 13670, :display_name "Description", :database_required false}) ``` AKA, exploded. But we cannot enter values for these fields for implicit actions. Before this change, the action thought its parameters were: ```clojure :parameters ({:id "json_val", :target [:variable [:template-tag "json_val"]], :type :type/Structured, :required true} {:id "json_val_%E2%86%92_a", :target [:variable [:template-tag "json_val_%E2%86%92_a"]], :type :type/Integer, :required false} {:id "json_val_%E2%86%92_b", :target [:variable [:template-tag "json_val_%E2%86%92_b"]], :type :type/Integer, :required false} {:id "description", :target [:variable [:template-tag "description"]], :type :type/Text, :required false}) ``` And we just don't know how to handle the exploded json values in the insert statement to create a nested json structure. So lets omit them. After this change: ```clojure action=> (:parameters (select-action :id 14)) ({:id "json_val", :target [:variable [:template-tag "json_val"]], :type :type/Structured, :required true} {:id "description", :target [:variable [:template-tag "description"]], :type :type/Text, :required false}) ``` * Translate `:type/Structured` to JSON for pg Fields have effective_type, and base_type `:type/Structured`. But several other types get marked as this as well: ```clojure actions=> (descendants :type/Structured) -> #{:type/SerializedJSON :type/XML :type/JSON} ``` So for the purposes of this, we _assume_ that structured is synonymous with json. Certainly will error if its xml. This makes the implicit action cast the text input for a json field to json. If the underlying field in pg is jsonb the driver cleans that up for us with no error. * Remove json fields, exploded json fields, unrecognized type fields In order to do this, had to change how we load json fields in test datasets for postgres. It just stuffed them in as text. Mysql would correctly make them json fields but postgres just bailed and left them as text strings. This changes `load-data/load-data!` to look for json columns, and if so, cast them to json when loading. * Remove json and xml checks from dashboard execution tests these are not doing what you think they are doing. they inspire confidence that json works but the underlying fields are actually always text. So it's unfounded confidence; misleading confidence. If you fix this, you quickly run into issues with expressivity in parameters and differences in dbs. A few: - no parameter on a dashboard can express that its type is json - mysql doesn't have an xml type - postgres does have an xml type, and you get back a native PGXML type - since you can't have an implicit type in the parameter, each sql has to cast in its query. And then you start getting differences in return types, queries, and it stops making sense for this all to be in the same query. And at the end of the day, of course you can write a query that casts a string to some native type. That's not really action related. * Let's not do this yet i think this is the correct call but want more tests on it when we flip the switch * Test pg enums in implicit actions * alignment and unused binding * Exclude in maria maria doesn't see the json_bit field as a json column. It's just a text column. So it is not excluded and all of the exploded fields like "json_bit → 1234" are not extracted. So this test is not applicable because it's just a generic implicit action on three fields. * accept suggestions from tamas
Showing
- src/metabase/driver/postgres/actions.clj 1 addition, 0 deletionssrc/metabase/driver/postgres/actions.clj
- src/metabase/models/action.clj 8 additions, 0 deletionssrc/metabase/models/action.clj
- test/metabase/api/dashboard_test.clj 0 additions, 2 deletionstest/metabase/api/dashboard_test.clj
- test/metabase/driver/postgres_test.clj 30 additions, 0 deletionstest/metabase/driver/postgres_test.clj
- test/metabase/models/action_test.clj 34 additions, 7 deletionstest/metabase/models/action_test.clj
- test/metabase/test/data/postgres.clj 22 additions, 3 deletionstest/metabase/test/data/postgres.clj
Loading
Please register or sign in to comment