-
- Downloads
MBQL Refactor: Combine various Field clauses into one new clause (#14897)
Background The original version of MBQL (known as the "Structured Query" language at the time, which was a little confusing) just referred to all fields by integer ID. e.g. {:filter ["STARTS_WITH" 1 "abc"]} That made clauses like this ambiguous: {:filter ["=" 1 2]} ; is 2 a Field, or the number 2? To clear up ambiguity and to let you filter Fields against Fields, we added the :field-id clause to make it explicit that you were referring to a Field, not an Integer: {:filter [:= [:field-id 1] [:field-id 2]]} ; Field 1 == Field 2 We soon added a couple of new types of Field clauses, :fk-> and :datetime-field, both of which wrap the original :field-id: ;; refer to Field 2 from a different Table, use Field 1 from the current Table to perform the join [:fk-> [:field-id 1] [:field-id 2]] ;; bucket Field 3 by month [:datetime-field [:field 3] :month] So far things were still pretty reasonable, the worst you'd have to do is something like [:datetime-field [:fk-> [:field-id 1] [:field-id 2]] :month] Things started to get out-of-hand IMO when we continued to add more Field clause types that just wrapped everything else. We added :binning-strategy: [:binning-strategy <field> :num-buckets 10] then :field-literal, to refer to a Field from a nested native source query: [:field-literal "my_field" :type/Text] then we added :joined-field to specify the Table you're explicitly joining against that is the source of a Field: [:joined-field "my_join" [:field-id 4]] This ends up getting really hairy when you combine things. This is an actual real clause you can use right now: [:binning-strategy [:datetime-field [:joined-field "my_join" [:field-literal "my_field" :type/DateTimeWithLocalTZ]] :month] :num-bins 10] And even with all of those clauses, we still can't pass around arbitrary extra information in a way that would make it easy to implement performance optimizations. If we ever try to add another new clause, the whole house of cards is going to come crashing down. The proposal Combine all of the Field clauses into a single new :field clauses with the schema [:field id-or-name options-map] Here are some before & after examples: [:field-id 1] => [:field 1 nil] [:field-literal "my_field" :type/Text] => [:field "my_field" {:base-type :type/Text}] [:datetime-field [:field 1] :month] => [:field 1 {:temporal-unit :month}] [:binning-strategy [:field 1] :num-bins 10] => [:field 1 {:binning {:strategy :num-bins, :num-bins 10}}] [:joined-field "my_join" [:field 1]] => [:field 1 {:join-alias "my_join}] [:fk-> [:field 1] [:field 2]] => [:field 2 {:source-field 1}] [:binning-strategy [:datetime-field [:joined-field "my_join" [:field-literal "my_field" :type/DateTimeWithLocalTZ]] :month] :num-bins 10] => [:field "my_field" {:base-type :type/DateTimeWithLocalTZ, :join-alias "my_join", :binning {:strategy :num-bins, :num-bins 10}}]
Showing
- backend/mbql/src/metabase/mbql/normalize.clj 291 additions, 179 deletionsbackend/mbql/src/metabase/mbql/normalize.clj
- backend/mbql/src/metabase/mbql/predicates.clj 9 additions, 1 deletionbackend/mbql/src/metabase/mbql/predicates.clj
- backend/mbql/src/metabase/mbql/schema.clj 222 additions, 150 deletionsbackend/mbql/src/metabase/mbql/schema.clj
- backend/mbql/src/metabase/mbql/schema/helpers.clj 1 addition, 1 deletionbackend/mbql/src/metabase/mbql/schema/helpers.clj
- backend/mbql/src/metabase/mbql/util.clj 85 additions, 89 deletionsbackend/mbql/src/metabase/mbql/util.clj
- backend/mbql/test/metabase/mbql/normalize_test.clj 319 additions, 112 deletionsbackend/mbql/test/metabase/mbql/normalize_test.clj
- backend/mbql/test/metabase/mbql/schema_test.clj 32 additions, 0 deletionsbackend/mbql/test/metabase/mbql/schema_test.clj
- backend/mbql/test/metabase/mbql/util_test.clj 412 additions, 435 deletionsbackend/mbql/test/metabase/mbql/util_test.clj
- dev/src/dev/debug_qp.clj 93 additions, 25 deletionsdev/src/dev/debug_qp.clj
- enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj 1 addition, 1 deletion...ise/backend/src/metabase_enterprise/sandbox/api/table.clj
- enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj 1 addition, 5 deletions...e_enterprise/sandbox/models/group_table_access_policy.clj
- enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/column_level_perms_check.clj 7 additions, 2 deletions...x/query_processor/middleware/column_level_perms_check.clj
- enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj 16 additions, 12 deletions...box/query_processor/middleware/row_level_restrictions.clj
- enterprise/backend/src/metabase_enterprise/serialization/load.clj 5 additions, 1 deletion...se/backend/src/metabase_enterprise/serialization/load.clj
- enterprise/backend/src/metabase_enterprise/serialization/serialize.clj 3 additions, 8 deletions...ckend/src/metabase_enterprise/serialization/serialize.clj
- enterprise/backend/test/metabase_enterprise/sandbox/models/group_table_access_policy_test.clj 5 additions, 5 deletions...erprise/sandbox/models/group_table_access_policy_test.clj
- enterprise/backend/test/metabase_enterprise/sandbox/pulse_test.clj 4 additions, 4 deletions...e/backend/test/metabase_enterprise/sandbox/pulse_test.clj
- enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj 19 additions, 15 deletions...uery_processor/middleware/row_level_restrictions_test.clj
- enterprise/backend/test/metabase_enterprise/sandbox/test_util.clj 4 additions, 4 deletions...se/backend/test/metabase_enterprise/sandbox/test_util.clj
- enterprise/backend/test/metabase_enterprise/serialization/test_util.clj 4 additions, 4 deletions...kend/test/metabase_enterprise/serialization/test_util.clj
Loading
Please register or sign in to comment