From e2ad1106ff0b075337de0673a734dd8bb2890561 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Thu, 6 Apr 2023 20:16:06 -0700
Subject: [PATCH] MLv2: Column metadata include `:lib/source-column-alias` and
`:lib/desired-column-alias` (#29824)
* MLv2 source-alias and desired-alias
* Add tests for JVM metadata provider
* De-duplicate JVM versions
* Remove unused
* Appease Kondo
* Address PR feedback
* Test fixes :wrench:
* Fix padding logic.
* Extra test cases
---
.../src/metabase-lib/order_by.unit.spec.ts | 6 +-
frontend/test/jest-setup.js | 7 +
package.json | 1 +
src/metabase/driver/impl.clj | 34 +--
src/metabase/lib/field.cljc | 57 ++++-
src/metabase/lib/join.cljc | 12 +-
src/metabase/lib/metadata.cljc | 11 +-
src/metabase/lib/metadata/calculation.cljc | 2 +-
src/metabase/lib/metadata/jvm.clj | 4 +-
src/metabase/lib/query.cljc | 2 +-
src/metabase/lib/stage.cljc | 223 +++++++++++------
src/metabase/lib/util.cljc | 84 ++++++-
test/metabase/api/metric_test.clj | 2 +-
test/metabase/api/segment_test.clj | 2 +-
test/metabase/driver/impl_test.clj | 81 +------
test/metabase/lib/aggregation_test.cljc | 32 +--
test/metabase/lib/expression_test.cljc | 4 +-
test/metabase/lib/field_test.cljc | 35 +++
test/metabase/lib/metadata/jvm_test.clj | 30 +++
test/metabase/lib/order_by_test.cljc | 224 ++++++++++++------
test/metabase/lib/stage_test.cljc | 81 +++++--
test/metabase/lib/util_test.cljc | 90 +++++++
test/metabase/models/metric_test.clj | 4 +-
test/metabase/models/segment_test.clj | 2 +-
.../explicit_joins_test.clj | 2 +-
yarn.lock | 5 +
26 files changed, 709 insertions(+), 328 deletions(-)
create mode 100644 test/metabase/lib/metadata/jvm_test.clj
diff --git a/frontend/src/metabase-lib/order_by.unit.spec.ts b/frontend/src/metabase-lib/order_by.unit.spec.ts
index 03c7d545afc..04726c23591 100644
--- a/frontend/src/metabase-lib/order_by.unit.spec.ts
+++ b/frontend/src/metabase-lib/order_by.unit.spec.ts
@@ -122,7 +122,9 @@ describe("order by", () => {
const orderBys = ML.orderBys(nextQuery);
expect(orderBys).toHaveLength(1);
- expect(ML.displayName(nextQuery, orderBys[0])).toBe("Title ascending");
+ expect(ML.displayName(nextQuery, orderBys[0])).toBe(
+ "Products → Title ascending",
+ );
});
});
@@ -148,7 +150,7 @@ describe("order by", () => {
);
const nextOrderBys = ML.orderBys(nextQuery);
expect(ML.displayName(nextQuery, nextOrderBys[0])).toBe(
- "Category descending",
+ "Products → Category descending",
);
expect(orderBys[0]).not.toEqual(nextOrderBys[0]);
});
diff --git a/frontend/test/jest-setup.js b/frontend/test/jest-setup.js
index bc4b5ae771a..97acb8023ce 100644
--- a/frontend/test/jest-setup.js
+++ b/frontend/test/jest-setup.js
@@ -1,3 +1,4 @@
+import { TextEncoder, TextDecoder } from "util";
import "cross-fetch/polyfill";
import "raf/polyfill";
import "jest-localstorage-mock";
@@ -23,3 +24,9 @@ if (process.env["DISABLE_LOGGING"] || process.env["DISABLE_LOGGING_FRONTEND"]) {
trace: jest.fn(),
};
}
+
+// global TextEncoder is not available in jsdom + Jest, see
+// https://stackoverflow.com/questions/70808405/how-to-set-global-textdecoder-in-jest-for-jsdom-if-nodes-util-textdecoder-is-ty
+// (hacky fix)
+global.TextEncoder = TextEncoder;
+global.TextDecoder = TextDecoder;
diff --git a/package.json b/package.json
index d12c4067ae9..33ff723dfbc 100644
--- a/package.json
+++ b/package.json
@@ -33,6 +33,7 @@
"classnames": "^2.1.3",
"color": "^3.0.0",
"color-harmony": "^0.3.0",
+ "crc-32": "^1.2.2",
"cron-expression-validator": "^1.0.20",
"cronstrue": "^2.11.0",
"crossfilter": "^1.3.12",
diff --git a/src/metabase/driver/impl.clj b/src/metabase/driver/impl.clj
index 85d7ca6dce2..984345c8b18 100644
--- a/src/metabase/driver/impl.clj
+++ b/src/metabase/driver/impl.clj
@@ -2,6 +2,7 @@
"Internal implementation functions for [[metabase.driver]]. These functions live in a separate namespace to reduce the
clutter in [[metabase.driver]] itself."
(:require
+ [metabase.lib.util :as lib.util]
[metabase.plugins.classloader :as classloader]
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
@@ -216,23 +217,6 @@
;;; ----------------------------------------------- [[truncate-alias]] -----------------------------------------------
-;; To truncate a string to a number of bytes we just iterate thru it character-by-character and keep a cumulative count
-;; of the total number of bytes up to the current character. Once we exceed `max-length-bytes` we return the substring
-;; from the character before we went past the limit.
-(defn- truncate-string-to-byte-count
- "Truncate string `s` to `max-length-bytes` UTF-8 bytes (as opposed to truncating to some number of *characters*)."
- ^String [^String s max-length-bytes]
- {:pre [(not (neg? max-length-bytes))]}
- (loop [i 0, cumulative-byte-count 0]
- (cond
- (= cumulative-byte-count max-length-bytes) (subs s 0 i)
- (> cumulative-byte-count max-length-bytes) (subs s 0 (dec i))
- (>= i (count s)) s
- :else (recur (inc i)
- (+
- cumulative-byte-count
- (count (.getBytes (str (.charAt s i)) "UTF-8")))))))
-
(def default-alias-max-length-bytes
"Default length to truncate column and table identifiers to for the default implementation
of [[metabase.driver/escape-alias]]."
@@ -241,11 +225,6 @@
;; identifiers we generate to 60 bytes so we have room to add `_2` and stuff without drama
60)
-(def ^:private truncated-alias-hash-suffix-length
- "Length of the hash suffixed to truncated strings by [[truncate-alias]]."
- ;; 8 bytes for the CRC32 plus one for the underscore
- 9)
-
(defn truncate-alias
"Truncate string `s` if it is longer than `max-length-bytes` (default [[default-alias-max-length-bytes]]) and append a
hex-encoded CRC-32 checksum of the original string. Truncated string is truncated to `max-length-bytes`
@@ -258,13 +237,4 @@
(truncate-alias s default-alias-max-length-bytes))
(^String [^String s max-length-bytes]
- ;; we can't truncate to something SHORTER than the suffix length. This precondition is here mostly to make sure
- ;; driver authors don't try to do something INSANE -- it shouldn't get hit during normal usage if a driver is
- ;; implemented properly.
- {:pre [(string? s) (integer? max-length-bytes) (> max-length-bytes truncated-alias-hash-suffix-length)]}
- (if (<= (count (.getBytes s "UTF-8")) max-length-bytes)
- s
- (let [checksum (Long/toHexString (.getValue (doto (java.util.zip.CRC32.)
- (.update (.getBytes s "UTF-8")))))
- truncated (truncate-string-to-byte-count s (- max-length-bytes truncated-alias-hash-suffix-length))]
- (str truncated \_ checksum)))))
+ (lib.util/truncate-alias s max-length-bytes)))
diff --git a/src/metabase/lib/field.cljc b/src/metabase/lib/field.cljc
index a9497c1f0ce..5f00b3b98f8 100644
--- a/src/metabase/lib/field.cljc
+++ b/src/metabase/lib/field.cljc
@@ -90,8 +90,8 @@
(lib.metadata.calculation/type-of query stage-number (resolve-field-metadata query stage-number field-ref)))
(defmethod lib.metadata.calculation/metadata-method :metadata/field
- [_query _stage-number field-metadata]
- field-metadata)
+ [_query _stage-number {field-name :name, :as field-metadata}]
+ (assoc field-metadata :name field-name))
;;; TODO -- base type should be affected by `temporal-unit`, right?
(defmethod lib.metadata.calculation/metadata-method :field
@@ -122,12 +122,18 @@
field-name :name
temporal-unit :unit
join-alias :source_alias
+ fk-field-id :fk_field_id
+ table-id :table_id
:as _field-metadata}]
(let [field-display-name (or field-display-name
(u.humanization/name->human-readable-name :simple field-name))
- join-display-name (when join-alias
- (let [join (lib.join/resolve-join query stage-number join-alias)]
- (lib.metadata.calculation/display-name query stage-number join)))
+ join-display-name (or
+ (when fk-field-id
+ (let [table (lib.metadata/table query table-id)]
+ (lib.metadata.calculation/display-name query stage-number table)))
+ (when join-alias
+ (let [join (lib.join/resolve-join query stage-number join-alias)]
+ (lib.metadata.calculation/display-name query stage-number join))))
display-name (if join-display-name
(str join-display-name " → " field-display-name)
field-display-name)]
@@ -136,14 +142,26 @@
display-name)))
(defmethod lib.metadata.calculation/display-name-method :field
- [query stage-number [_field {:keys [join-alias temporal-unit], :as _opts} _id-or-name, :as field-clause]]
+ [query stage-number [_tag {:keys [join-alias temporal-unit source-field], :as _opts} _id-or-name, :as field-clause]]
(if-let [field-metadata (cond-> (resolve-field-metadata query stage-number field-clause)
join-alias (assoc :source_alias join-alias)
- temporal-unit (assoc :unit temporal-unit))]
+ temporal-unit (assoc :unit temporal-unit)
+ source-field (assoc :fk_field_id source-field))]
(lib.metadata.calculation/display-name query stage-number field-metadata)
;; mostly for the benefit of JS, which does not enforce the Malli schemas.
(i18n/tru "[Unknown Field]")))
+(defmethod lib.metadata.calculation/column-name-method :metadata/field
+ [_query _stage-number {field-name :name}]
+ field-name)
+
+(defmethod lib.metadata.calculation/column-name-method :field
+ [query stage-number [_tag _id-or-name, :as field-clause]]
+ (if-let [field-metadata (resolve-field-metadata query stage-number field-clause)]
+ (lib.metadata.calculation/column-name query stage-number field-metadata)
+ ;; mostly for the benefit of JS, which does not enforce the Malli schemas.
+ "unknown_field"))
+
(defmethod lib.temporal-bucket/current-temporal-bucket-method :field
[[_tag opts _id-or-name]]
(:temporal-unit opts))
@@ -206,6 +224,31 @@
(:name metadata)
(or (:id metadata) (:name metadata)))])))
+(mu/defn ^:private joined-field-desired-alias :- ::lib.schema.common/non-blank-string
+ "Desired alias for a Field that comes from a join, e.g.
+
+ MyJoin__my_field
+
+ You should pass the results thru a unique name function."
+ [join-alias :- ::lib.schema.common/non-blank-string
+ field-name :- ::lib.schema.common/non-blank-string]
+ (lib.util/format "%s__%s" join-alias field-name))
+
+(mu/defn desired-alias :- ::lib.schema.common/non-blank-string
+ "Desired alias for a Field e.g.
+
+ my_field
+
+ OR
+
+ MyJoin__my_field
+
+ You should pass the results thru a unique name function."
+ [field-metadata :- lib.metadata/ColumnMetadata]
+ (if-let [join-alias (lib.join/current-join-alias field-metadata)]
+ (joined-field-desired-alias join-alias (:name field-metadata))
+ (:name field-metadata)))
+
(defn fields
"Specify the `:fields` for a query."
([xs]
diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc
index 5fd5b11c5f5..a06d61e4256 100644
--- a/src/metabase/lib/join.cljc
+++ b/src/metabase/lib/join.cljc
@@ -85,7 +85,7 @@
(let [column-metadata (assoc column-metadata :source_alias join-alias)
col (-> (assoc column-metadata
:display_name (lib.metadata.calculation/display-name query stage-number column-metadata)
- :lib/source :source/fields)
+ :lib/source :source/joins)
(with-join-alias join-alias))]
(assert (= (current-join-alias col) join-alias))
col))
@@ -243,3 +243,13 @@
([query :- ::lib.schema/query
stage-number :- ::lib.schema.common/int-greater-than-or-equal-to-zero]
(not-empty (get (lib.util/query-stage query stage-number) :joins))))
+
+(mu/defn implicit-join-name :- ::lib.schema.common/non-blank-string
+ "Name for an implicit join against `table-name` via an FK field, e.g.
+
+ CATEGORIES__via__CATEGORY_ID
+
+ You should make sure this gets ran thru a unique-name fn."
+ [table-name :- ::lib.schema.common/non-blank-string
+ source-field-id-name :- ::lib.schema.common/non-blank-string]
+ (lib.util/format "%s__via__%s" table-name source-field-id-name))
diff --git a/src/metabase/lib/metadata.cljc b/src/metabase/lib/metadata.cljc
index 4bb03a65dcd..652c4c4715b 100644
--- a/src/metabase/lib/metadata.cljc
+++ b/src/metabase/lib/metadata.cljc
@@ -85,7 +85,16 @@
[:source_alias {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
;; what top-level clause in the query this metadata originated from, if it is calculated (i.e., if this metadata
;; was generated by [[metabase.lib.metadata.calculation/metadata]])
- [:lib/source {:optional true} [:ref ::column-source]]])
+ [:lib/source {:optional true} [:ref ::column-source]]
+ ;;
+ ;; this stuff is adapted from [[metabase.query-processor.util.add-alias-info]]. It is included in
+ ;; the [[metabase.lib.metadata.calculation/metadata]]
+ ;;
+ ;; the alias that should be used to this clause on the LHS of a `SELECT <lhs> AS <rhs>` or equivalent, i.e. the
+ ;; name of this clause as exported by the previous stage, source table, or join.
+ [:lib/source-column-alias {:optional true} [:maybe ::lib.schema.common/non-blank-string]]
+ ;; the name we should export this column as, i.e. the RHS of a `SELECT <lhs> AS <rhs>` or equivalent.
+ [:lib/desired-column-alias {:optional true} [:maybe ::lib.schema.common/non-blank-string]]])
(def ^:private CardMetadata
[:map
diff --git a/src/metabase/lib/metadata/calculation.cljc b/src/metabase/lib/metadata/calculation.cljc
index 803f2c5800c..060903ee48e 100644
--- a/src/metabase/lib/metadata/calculation.cljc
+++ b/src/metabase/lib/metadata/calculation.cljc
@@ -92,7 +92,7 @@
(defn- slugify [s]
(-> s
(str/replace #"[\(\)]" "")
- u/slugify))
+ (u/slugify {:unicode? true})))
;;; default impl just takes the display name and slugifies it.
(defmethod column-name-method :default
diff --git a/src/metabase/lib/metadata/jvm.clj b/src/metabase/lib/metadata/jvm.clj
index ee8b75c05c7..f1b2ae30943 100644
--- a/src/metabase/lib/metadata/jvm.clj
+++ b/src/metabase/lib/metadata/jvm.clj
@@ -1,4 +1,4 @@
-(ns metabase.lib.metadata.jvm
+ (ns metabase.lib.metadata.jvm
"Implementation(s) of [[metabase.lib.metadata.protocols/MetadataProvider]] only for the JVM."
(:require
[metabase.lib.metadata.cached-provider :as lib.metadata.cached-provider]
@@ -59,7 +59,7 @@
(fields [_this table-id]
(log/debugf "Fetching all Fields for Table %d" table-id)
(mapv #(assoc % :lib/type :metadata/field)
- (t2/select :table_id table-id)))
+ (t2/select :metabase.models.field/Field :table_id table-id)))
lib.metadata.protocols/BulkMetadataProvider
(bulk-metadata [_this metadata-type ids]
diff --git a/src/metabase/lib/query.cljc b/src/metabase/lib/query.cljc
index 45a089b5034..e38cf72089f 100644
--- a/src/metabase/lib/query.cljc
+++ b/src/metabase/lib/query.cljc
@@ -146,7 +146,7 @@
"Convenience for creating a query from a Saved Question (i.e., a Card)."
[metadata-provider :- lib.metadata/MetadataProvider
{mbql-query :dataset_query, metadata :result_metadata}]
- (let [mbql-query (cond-> (assoc (lib.util/pipeline mbql-query)
+ (let [mbql-query (cond-> (assoc (lib.convert/->pMBQL mbql-query)
:lib/metadata metadata-provider)
metadata
(lib.util/update-query-stage -1 assoc :lib/stage-metadata metadata))]
diff --git a/src/metabase/lib/stage.cljc b/src/metabase/lib/stage.cljc
index 8582bb7f63d..72cc06dc597 100644
--- a/src/metabase/lib/stage.cljc
+++ b/src/metabase/lib/stage.cljc
@@ -6,6 +6,8 @@
[metabase.lib.aggregation :as lib.aggregation]
[metabase.lib.breakout :as lib.breakout]
[metabase.lib.expression :as lib.expression]
+ [metabase.lib.field :as lib.field]
+ [metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.normalize :as lib.normalize]
@@ -22,6 +24,14 @@
(declare stage-metadata)
+(defn- unique-name-generator []
+ (comp (mbql.u/unique-name-generator
+ ;; unique by lower-case name, e.g. `NAME` and `name` => `NAME` and `name_2`
+ :name-key-fn u/lower-case-en
+ ;; truncate alias to 30 characters (actually 21 characters plus a hash).
+ :unique-alias-fn (fn [original suffix]
+ (lib.util/truncate-alias (str original \_ suffix))))))
+
(defmethod lib.normalize/normalize :mbql.stage/mbql
[stage]
(lib.normalize/normalize-map
@@ -41,22 +51,24 @@
(lib.util/update-query-stage previous-stage-number
assoc
::cached-metadata
- {:lib/type :metadata/results
- :columns (stage-metadata query previous-stage-number)}))))
+ (stage-metadata query previous-stage-number)))))
(def ^:private StageMetadataColumns
- [:sequential {:min 1} lib.metadata.calculation/ColumnMetadataWithSource])
-
-(def ^:private DistinctStageMetadataColumns
[:and
- StageMetadataColumns
+ [:sequential {:min 1}
+ [:merge
+ lib.metadata.calculation/ColumnMetadataWithSource
+ [:map
+ [:lib/source-column-alias ::lib.schema.common/non-blank-string]
+ [:lib/desired-column-alias ::lib.schema.common/non-blank-string]]]]
[:fn
;; should be dev-facing only, so don't need to i18n
- {:error/message "Column :names must be distinct!"
+ {:error/message "Column :lib/desired-column-alias values must be distinct, regardless of case, for each stage!"
:error/fn (fn [{:keys [value]} _]
- (str "Column :names must be distinct, got: " (pr-str (mapv :name value))))}
+ (str "Column :lib/desired-column-alias values must be distinct, got: "
+ (pr-str (mapv :lib/desired-column-alias value))))}
(fn [columns]
- (apply distinct? (map :name columns)))]])
+ (apply distinct? (map (comp u/lower-case-en :lib/desired-column-alias) columns)))]])
(mu/defn ^:private existing-stage-metadata :- [:maybe StageMetadataColumns]
"Return existing stage metadata attached to a stage if is already present: return it as-is, but only if this is a
@@ -76,41 +88,71 @@
(assoc col :lib/source source-type))))))))
(mu/defn ^:private breakouts-columns :- [:maybe StageMetadataColumns]
- [query :- ::lib.schema/query
- stage-number :- :int]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
(not-empty
(for [breakout (lib.breakout/breakouts query stage-number)]
- (assoc breakout :lib/source :source/breakouts))))
+ (assoc breakout
+ :lib/source :source/breakouts
+ :lib/source-column-alias (:name breakout)
+ :lib/desired-column-alias (unique-name-fn (:name breakout))))))
(mu/defn ^:private aggregations-columns :- [:maybe StageMetadataColumns]
- [query :- ::lib.schema/query
- stage-number :- :int]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
(not-empty
(for [ag (lib.aggregation/aggregations query stage-number)]
- (assoc ag :lib/source :source/aggregations))))
+ (assoc ag
+ :lib/source :source/aggregations
+ :lib/source-column-alias (:name ag)
+ :lib/desired-column-alias (unique-name-fn (:name ag))))))
(mu/defn ^:private fields-columns :- [:maybe StageMetadataColumns]
- [query :- ::lib.schema/query
- stage-number :- :int]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
(when-let [{fields :fields} (lib.util/query-stage query stage-number)]
(not-empty
- (for [[tag :as ref-clause] fields]
- (assoc (lib.metadata.calculation/metadata query stage-number ref-clause)
- :lib/source (case tag
- ;; you can't have an `:aggregation` reference in `:fields`; anything in `:aggregations` is
- ;; returned automatically anyway by [[aggregations-columns]] above.
- :field :source/fields
- :expression :source/expressions))))))
+ (for [[tag :as ref-clause] fields
+ :let [source (case tag
+ ;; you can't have an `:aggregation` reference in `:fields`; anything in
+ ;; `:aggregations` is returned automatically anyway
+ ;; by [[aggregations-columns]] above.
+ :field :source/fields
+ :expression :source/expressions)
+ metadata (lib.metadata.calculation/metadata query stage-number ref-clause)]]
+ (assoc metadata
+ :lib/source source
+ :lib/source-column-alias (lib.metadata.calculation/column-name query stage-number metadata)
+ :lib/desired-column-alias (unique-name-fn (lib.field/desired-alias metadata)))))))
(mu/defn ^:private breakout-ags-fields-columns :- [:maybe StageMetadataColumns]
- [query :- ::lib.schema/query
- stage-number :- :int]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
(not-empty
(into []
- cat
- [(breakouts-columns query stage-number)
- (aggregations-columns query stage-number)
- (fields-columns query stage-number)])))
+ (mapcat (fn [f]
+ (f query stage-number unique-name-fn)))
+ [breakouts-columns
+ aggregations-columns
+ fields-columns])))
+
+(mu/defn ^:private previous-stage-metadata :- [:maybe StageMetadataColumns]
+ "Metadata for the previous stage, if there is one."
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
+ (when-let [previous-stage-number (lib.util/previous-stage-number query stage-number)]
+ (for [col (stage-metadata query previous-stage-number)
+ :let [source-alias (or ((some-fn :lib/desired-column-alias :lib/source-column-alias) col)
+ (lib.metadata.calculation/column-name query stage-number col))]]
+ (assoc col
+ :lib/source :source/previous-stage
+ :lib/source-column-alias source-alias
+ :lib/desired-column-alias (unique-name-fn source-alias)))))
(defn- remove-hidden-default-fields
"Remove Fields that shouldn't be visible from the default Fields for a source Table.
@@ -131,14 +173,18 @@
(mu/defn ^:private source-table-default-fields :- [:maybe StageMetadataColumns]
"Determine the Fields we'd normally return for a source Table.
See [[metabase.query-processor.middleware.add-implicit-clauses/add-implicit-fields]]."
- [query :- ::lib.schema/query
- table-id :- ::lib.schema.id/table]
+ [query :- ::lib.schema/query
+ table-id :- ::lib.schema.id/table
+ unique-name-fn :- fn?]
(when-let [field-metadatas (lib.metadata/fields query table-id)]
(->> field-metadatas
remove-hidden-default-fields
sort-default-fields
(map (fn [col]
- (assoc col :lib/source :source/table-defaults))))))
+ (assoc col
+ :lib/source :source/table-defaults
+ :lib/source-column-alias (:name col)
+ :lib/desired-column-alias (unique-name-fn (:name col))))))))
(mu/defn ^:private default-join-alias :- ::lib.schema.common/non-blank-string
"Generate an alias for a join that doesn't already have one."
@@ -160,33 +206,39 @@
[query :- ::lib.schema/query
stage-number :- :int
joins :- ::lib.schema.join/joins]
- (let [unique-name-generator (mbql.u/unique-name-generator)]
+ (let [unique-name-fn (unique-name-generator)]
(mapv (fn [join]
(cond-> join
- (not (:alias join)) (assoc :alias (unique-name-generator (default-join-alias query stage-number join)))))
+ (not (:alias join)) (assoc :alias (unique-name-fn (default-join-alias query stage-number join)))))
joins)))
(mu/defn ^:private default-columns-added-by-join :- [:maybe StageMetadataColumns]
- [query :- ::lib.schema/query
- stage-number :- :int
- join :- ::lib.schema.join/join]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?
+ join :- ::lib.schema.join/join]
+ (assert (:alias join) "Join must have :alias")
(not-empty
(for [col (lib.metadata.calculation/metadata query stage-number join)]
- (assoc col :lib/source :source/joins))))
+ (assoc col
+ :lib/source-column-alias (lib.metadata.calculation/column-name query stage-number col)
+ :lib/desired-column-alias (unique-name-fn (lib.field/desired-alias col))))))
(mu/defn ^:private default-columns-added-by-joins :- [:maybe StageMetadataColumns]
- [query :- ::lib.schema/query
- stage-number :- :int]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
(when-let [joins (not-empty (:joins (lib.util/query-stage query stage-number)))]
(not-empty
(into []
- (mapcat (partial default-columns-added-by-join query stage-number))
+ (mapcat (partial default-columns-added-by-join query stage-number unique-name-fn))
(ensure-all-joins-have-aliases query stage-number joins)))))
(mu/defn ^:private saved-question-metadata :- [:maybe StageMetadataColumns]
"Metadata associated with a Saved Question, if `:source-table` is a `card__<id>` string."
- [query :- ::lib.schema/query
- source-table-id]
+ [query :- ::lib.schema/query
+ source-table-id :- [:or ::lib.schema.id/table ::lib.schema.id/table-card-id-string]
+ unique-name-fn :- fn?]
(when-let [card-id (lib.util/string-table-id->card-id source-table-id)]
;; it seems like in some cases the FE is renaming `:result_metadata` to `:fields`, not 100% sure why but
;; handle that case anyway. (#29739)
@@ -195,7 +247,21 @@
(map? result-metadata) (:columns result-metadata)
(sequential? result-metadata) result-metadata))]
(for [col cols]
- (assoc col :lib/source :source/card))))))
+ (assoc col
+ :lib/source :source/card
+ :lib/source-column-alias (:name col)
+ :lib/desired-column-alias (unique-name-fn (:name col))))))))
+
+(mu/defn ^:private expressions-metadata :- [:maybe StageMetadataColumns]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
+ (not-empty
+ (for [expression (lib.expression/expressions query stage-number)]
+ (assoc expression
+ :lib/source :source/expressions
+ :lib/source-column-alias (:name expression)
+ :lib/desired-column-alias (unique-name-fn (:name expression))))))
(mu/defn ^:private default-columns :- StageMetadataColumns
"Calculate the columns to return if `:aggregations`/`:breakout`/`:fields` are unspecified.
@@ -217,50 +283,43 @@
PLUS
3. Columns added by joins at this stage"
- [query :- ::lib.schema/query
- stage-number :- :int]
+ [query :- ::lib.schema/query
+ stage-number :- :int
+ unique-name-fn :- fn?]
(concat
;; 1: columns from the previous stage, source table or query
- (if-let [previous-stage-number (lib.util/previous-stage-number query stage-number)]
+ (or
;; 1a. columns returned by previous stage
- (for [col (stage-metadata query previous-stage-number)]
- (assoc col :lib/source :source/previous-stage))
+ (previous-stage-metadata query stage-number unique-name-fn)
;; 1b or 1c
(let [{:keys [source-table], :as this-stage} (lib.util/query-stage query stage-number)]
(or
;; 1b: default visible Fields for the source Table
(when (integer? source-table)
- (source-table-default-fields query source-table))
+ (source-table-default-fields query source-table unique-name-fn))
;; 1c. Metadata associated with a saved Question
- (saved-question-metadata query source-table)
+ (saved-question-metadata query source-table unique-name-fn)
;; 1d: `:lib/stage-metadata` for the (presumably native) query
(for [col (:columns (:lib/stage-metadata this-stage))]
(assoc col :lib/source :source/native)))))
;; 2: expressions (aka calculated columns) added in this stage
- (lib.expression/expressions query stage-number)
+ (expressions-metadata query stage-number unique-name-fn)
;; 3: columns added by joins at this stage
- (default-columns-added-by-joins query stage-number)))
+ (default-columns-added-by-joins query stage-number unique-name-fn)))
-(defn- ensure-distinct-names [metadatas]
- (when (seq metadatas)
- (let [f (mbql.u/unique-name-generator)]
- (for [metadata metadatas]
- (update metadata :name f)))))
-
-(mu/defn ^:private stage-metadata :- DistinctStageMetadataColumns
+(mu/defn ^:private stage-metadata :- StageMetadataColumns
"Return results metadata about the expected columns in an MBQL query stage. If the query has
aggregations/breakouts/fields, then return THOSE. Otherwise return the defaults based on the source Table or
previous stage + joins."
[query :- ::lib.schema/query
stage-number :- :int]
- (ensure-distinct-names
- (or
- (existing-stage-metadata query stage-number)
- (let [query (ensure-previous-stages-have-metadata query stage-number)]
- ;; ... then calculate metadata for this stage
- (or
- (breakout-ags-fields-columns query stage-number)
- (default-columns query stage-number))))))
+ (or
+ (existing-stage-metadata query stage-number)
+ (let [query (ensure-previous-stages-have-metadata query stage-number)]
+ ;; ... then calculate metadata for this stage
+ (or
+ (breakout-ags-fields-columns query stage-number (unique-name-generator))
+ (default-columns query stage-number (unique-name-generator))))))
(doseq [stage-type [:mbql.stage/mbql
:mbql.stage/native]]
@@ -298,7 +357,7 @@
it turns out we do need that stuff.
Does not include columns that would be implicitly joinable via multiple hops."
- [query column-metadatas]
+ [query column-metadatas unique-name-fn]
(let [existing-table-ids (into #{} (map :table_id) column-metadatas)]
(into []
(comp (filter :fk_target_field_id)
@@ -309,21 +368,31 @@
(remove #(contains? existing-table-ids (:table_id %)))
(m/distinct-by :table_id)
(mapcat (fn [{table-id :table_id, ::keys [source-field-id]}]
- (for [field (source-table-default-fields query table-id)]
- (assoc field :fk_field_id source-field-id))))
- (map (fn [metadata]
- (assoc metadata :lib/source :source/implicitly-joinable))))
+ (let [table-name (:name (lib.metadata/table query table-id))
+ source-field-id-name (:name (lib.metadata/field query source-field-id))
+ ;; make sure the implicit join name is unique.
+ source-alias (unique-name-fn
+ (lib.join/implicit-join-name table-name source-field-id-name))]
+ (for [field (source-table-default-fields query table-id unique-name-fn)
+ :let [field (assoc field
+ :fk_field_id source-field-id
+ :lib/source :source/implicitly-joinable
+ :lib/source-column-alias (:name field))
+ field (lib.join/with-join-alias field source-alias)]]
+ (assoc field :lib/desired-column-alias (unique-name-fn
+ (lib.field/desired-alias field))))))))
column-metadatas)))
(mu/defn visible-columns :- StageMetadataColumns
"Columns that are visible inside a given stage of a query. Ignores `:fields`, `:breakout`, and `:aggregation`.
Includes columns that are implicitly joinable from other Tables."
[query stage-number]
- (let [query (lib.util/update-query-stage query stage-number dissoc :fields :breakout :aggregation)
- columns (default-columns query stage-number)]
+ (let [query (lib.util/update-query-stage query stage-number dissoc :fields :breakout :aggregation)
+ unique-name-fn (unique-name-generator)
+ columns (default-columns query stage-number unique-name-fn)]
(concat
columns
- (implicitly-joinable-columns query columns))))
+ (implicitly-joinable-columns query columns unique-name-fn))))
(mu/defn append-stage :- ::lib.schema/query
"Adds a new blank stage to the end of the pipeline"
diff --git a/src/metabase/lib/util.cljc b/src/metabase/lib/util.cljc
index 18627fd0324..30c1a68512a 100644
--- a/src/metabase/lib/util.cljc
+++ b/src/metabase/lib/util.cljc
@@ -5,15 +5,20 @@
[clojure.string :as str]
[metabase.lib.options :as lib.options]
[metabase.lib.schema :as lib.schema]
+ [metabase.lib.schema.common :as lib.schema.common]
[metabase.lib.schema.id :as lib.schema.id]
[metabase.shared.util.i18n :as i18n]
[metabase.util.malli :as mu]
#?@(:clj
([potemkin :as p]))
#?@(:cljs
- ([goog.string :as gstring]
+ (["crc-32" :as CRC32]
+ [goog.string :as gstring]
[goog.string.format :as gstring.format]))))
+#?(:clj
+ (set! *warn-on-reflection* true))
+
;; The formatting functionality is only loaded if you depend on goog.string.format.
#?(:cljs (comment gstring.format/keep-me))
@@ -267,6 +272,83 @@
update-joins)]
(f query stage-number stage))))))
+(mu/defn ^:private string-byte-count :- [:int {:min 0}]
+ "Number of bytes in a string using UTF-8 encoding."
+ [s :- :string]
+ #?(:clj (count (.getBytes (str s) "UTF-8"))
+ :cljs (.. (js/TextEncoder.) (encode s) -length)))
+
+#?(:clj
+ (mu/defn ^:private string-character-at :- [:string {:min 0, :max 1}]
+ [s :- :string
+ i :-[:int {:min 0}]]
+ (str (.charAt ^String s i))))
+
+(mu/defn ^:private truncate-string-to-byte-count :- :string
+ "Truncate string `s` to `max-length-bytes` UTF-8 bytes (as opposed to truncating to some number of
+ *characters*)."
+ [s :- :string
+ max-length-bytes :- [:int {:min 1}]]
+ #?(:clj
+ (loop [i 0, cumulative-byte-count 0]
+ (cond
+ (= cumulative-byte-count max-length-bytes) (subs s 0 i)
+ (> cumulative-byte-count max-length-bytes) (subs s 0 (dec i))
+ (>= i (count s)) s
+ :else (recur (inc i)
+ (+
+ cumulative-byte-count
+ (string-byte-count (string-character-at s i))))))
+
+ :cljs
+ (let [buf (js/Uint8Array. max-length-bytes)
+ result (.encodeInto (js/TextEncoder.) s buf)] ;; JS obj {read: chars_converted, write: bytes_written}
+ (subs s 0 (.-read result)))))
+
+(def ^:private truncate-alias-max-length-bytes
+ "Length to truncate column and table identifiers to. See [[metabase.driver.impl/default-alias-max-length-bytes]] for
+ reasoning."
+ 60)
+
+(def ^:private truncated-alias-hash-suffix-length
+ "Length of the hash suffixed to truncated strings by [[truncate-alias]]."
+ ;; 8 bytes for the CRC32 plus one for the underscore
+ 9)
+
+(mu/defn ^:private crc32-checksum :- [:string {:min 8, :max 8}]
+ "Return a 4-byte CRC-32 checksum of string `s`, encoded as an 8-character hex string."
+ [s :- :string]
+ (let [s #?(:clj (Long/toHexString (.getValue (doto (java.util.zip.CRC32.)
+ (.update (.getBytes ^String s "UTF-8")))))
+ :cljs (-> (CRC32/str s 0)
+ (unsigned-bit-shift-right 0) ; see https://github.com/SheetJS/js-crc32#signed-integers
+ (.toString 16)))]
+ ;; pad to 8 characters if needed. Might come out as less than 8 if the first byte is `00` or `0x` or something.
+ (loop [s s]
+ (if (< (count s) 8)
+ (recur (str \0 s))
+ s))))
+
+(mu/defn truncate-alias :- ::lib.schema.common/non-blank-string
+ "Truncate string `s` if it is longer than [[truncate-alias-max-length-bytes]] and append a hex-encoded CRC-32
+ checksum of the original string. Truncated string is truncated to [[truncate-alias-max-length-bytes]]
+ minus [[truncated-alias-hash-suffix-length]] characters so the resulting string is
+ exactly [[truncate-alias-max-length-bytes]]. The goal here is that two really long strings that only differ at the
+ end will still have different resulting values.
+
+ (truncate-alias \"some_really_long_string\" 15) ; -> \"some_r_8e0f9bc2\"
+ (truncate-alias \"some_really_long_string_2\" 15) ; -> \"some_r_2a3c73eb\""
+ ([s]
+ (truncate-alias s truncate-alias-max-length-bytes))
+
+ ([s :- ::lib.schema.common/non-blank-string
+ max-bytes :- [:int {:min 0}]]
+ (if (<= (string-byte-count s) max-bytes)
+ s
+ (let [checksum (crc32-checksum s)
+ truncated (truncate-string-to-byte-count s (- max-bytes truncated-alias-hash-suffix-length))]
+ (str truncated \_ checksum)))))
+
(mu/defn string-table-id->card-id :- [:maybe ::lib.schema.id/card]
"If `table-id` is a `card__<id>`-style string, parse the `<id>` part to an integer Card ID."
[table-id]
diff --git a/test/metabase/api/metric_test.clj b/test/metabase/api/metric_test.clj
index c6159a8cec0..3d7b51a5109 100644
--- a/test/metabase/api/metric_test.clj
+++ b/test/metabase/api/metric_test.clj
@@ -412,7 +412,7 @@
{:name "Metric B"
:id id-2
:creator {}
- :definition_description "Venues, Sum of Name, Filtered by Price equals 4 and Segment"}]
+ :definition_description "Venues, Sum of Categories → Name, Filtered by Price equals 4 and Segment"}]
(filter (fn [{metric-id :id}]
(contains? #{id-1 id-2 id-3} metric-id))
(mt/user-http-request :rasta :get 200 "metric/")))))))
diff --git a/test/metabase/api/segment_test.clj b/test/metabase/api/segment_test.clj
index c00f22b7bb1..ecf9ce90224 100644
--- a/test/metabase/api/segment_test.clj
+++ b/test/metabase/api/segment_test.clj
@@ -425,7 +425,7 @@
:name "Segment 2"
:definition {}
:creator {}
- :definition_description "Filtered by Price equals 4 and Name equals \"BBQ\""}]
+ :definition_description "Filtered by Price equals 4 and Categories → Name equals \"BBQ\""}]
(filter (fn [{segment-id :id}]
(contains? #{id-1 id-2 id-3} segment-id))
(mt/user-http-request :rasta :get 200 "segment/")))))))
diff --git a/test/metabase/driver/impl_test.clj b/test/metabase/driver/impl_test.clj
index c4b5b0c5ae9..78f10362c05 100644
--- a/test/metabase/driver/impl_test.clj
+++ b/test/metabase/driver/impl_test.clj
@@ -1,7 +1,6 @@
(ns metabase.driver.impl-test
(:require
[clojure.core.async :as a]
- [clojure.string :as str]
[clojure.test :refer :all]
[metabase.driver :as driver]
[metabase.driver.impl :as driver.impl]
@@ -9,7 +8,7 @@
(set! *warn-on-reflection* true)
-(deftest driver->expected-namespace-test
+(deftest ^:parallel driver->expected-namespace-test
(testing "expected namespace for a non-namespaced driver should be `metabase.driver.<driver>`"
(is (= 'metabase.driver.sql-jdbc
(#'driver.impl/driver->expected-namespace :sql-jdbc))))
@@ -37,81 +36,3 @@
(driver/the-initialized-driver ::race-condition-test)))
(is (= true
@finished-loading)))))))
-
-(deftest truncate-string-to-byte-count-test
- (letfn [(truncate-string-to-byte-count [s byte-length]
- (let [^String truncated (#'driver.impl/truncate-string-to-byte-count s byte-length)]
- (is (<= (count (.getBytes truncated "UTF-8")) byte-length))
- (is (str/starts-with? s truncated))
- truncated))]
- (doseq [[s max-length->expected] {"12345"
- {0 ""
- 1 "1"
- 2 "12"
- 3 "123"
- 4 "1234"
- 5 "12345"
- 6 "12345"
- 10 "12345"}
-
- "가나다ë¼"
- {0 ""
- 1 ""
- 2 ""
- 3 "ê°€"
- 4 "ê°€"
- 5 "ê°€"
- 6 "가나"
- 7 "가나"
- 8 "가나"
- 9 "가나다"
- 10 "가나다"
- 11 "가나다"
- 12 "가나다ë¼"
- 13 "가나다ë¼"
- 15 "가나다ë¼"
- 20 "가나다ë¼"}}
- [max-length expected] max-length->expected]
- (testing (pr-str (list `driver.impl/truncate-string-to-byte-count s max-length))
- (is (= expected
- (truncate-string-to-byte-count s max-length)))))))
-
-(deftest truncate-alias-test
- (letfn [(truncate-alias [s max-bytes]
- (let [truncated (driver.impl/truncate-alias s max-bytes)]
- (is (<= (count (.getBytes truncated "UTF-8")) max-bytes))
- truncated))]
- (doseq [[s max-bytes->expected] { ;; 20-character plain ASCII string
- "01234567890123456789"
- {12 "012_fc89bad5"
- 15 "012345_fc89bad5"
- 20 "01234567890123456789"}
-
- ;; two strings that only differ after the point they get truncated
- "0123456789abcde" {12 "012_1629bb92"}
- "0123456789abcdE" {12 "012_2d479b5a"}
-
- ;; Unicode string: 14 characters, 42 bytes
- "가나다ë¼ë§ˆë°”사아ìžì°¨ì¹´íƒ€íŒŒí•˜"
- {12 "ê°€_b9c95392"
- 13 "ê°€_b9c95392"
- 14 "ê°€_b9c95392"
- 15 "가나_b9c95392"
- 20 "가나다_b9c95392"
- 30 "가나다ë¼ë§ˆë°”사_b9c95392"
- 40 "가나다ë¼ë§ˆë°”사아ìžì°¨_b9c95392"
- 50 "가나다ë¼ë§ˆë°”사아ìžì°¨ì¹´íƒ€íŒŒí•˜"}
-
- ;; Mixed string: 17 characters, 33 bytes
- "aê°€b나c다dë¼e마fë°”g사hì•„i"
- {12 "a_99a0fe0c"
- 13 "aê°€_99a0fe0c"
- 14 "aê°€b_99a0fe0c"
- 15 "aê°€b_99a0fe0c"
- 20 "a가b나c_99a0fe0c"
- 30 "aê°€b나c다dë¼e마f_99a0fe0c"
- 40 "aê°€b나c다dë¼e마fë°”g사hì•„i"}}
- [max-bytes expected] max-bytes->expected]
- (testing (pr-str (list `driver.impl/truncate-alias s max-bytes))
- (is (= expected
- (truncate-alias s max-bytes)))))))
diff --git a/test/metabase/lib/aggregation_test.cljc b/test/metabase/lib/aggregation_test.cljc
index 449eb278e50..7325f1c9041 100644
--- a/test/metabase/lib/aggregation_test.cljc
+++ b/test/metabase/lib/aggregation_test.cljc
@@ -50,10 +50,10 @@
{:column-name "count", :display-name "Count"}
[:distinct {} (lib.tu/field-clause :venues :id)]
- {:column-name "distinct_id", :display-name "Distinct values of ID"}
+ {:column-name "distinct_ID", :display-name "Distinct values of ID"}
[:sum {} (lib.tu/field-clause :venues :id)]
- {:column-name "sum_id", :display-name "Sum of ID"}
+ {:column-name "sum_ID", :display-name "Sum of ID"}
[:+ {} [:count {}] 1]
{:column-name "count_plus_1", :display-name "Count + 1"}
@@ -62,7 +62,7 @@
{}
[:min {} (lib.tu/field-clause :venues :id)]
[:* {} 2 [:avg {} (lib.tu/field-clause :venues :price)]]]
- {:column-name "min_id_plus_2_times_avg_price"
+ {:column-name "min_ID_plus_2_times_avg_PRICE"
:display-name "Min of ID + (2 × Average of Price)"}
[:+
@@ -74,7 +74,7 @@
[:avg {} (lib.tu/field-clause :venues :price)]
3
[:- {} [:max {} (lib.tu/field-clause :venues :category-id)] 4]]]
- {:column-name "min_id_plus_2_times_avg_price_times_3_times_max_category_id_minus_4"
+ {:column-name "min_ID_plus_2_times_avg_PRICE_times_3_times_max_CATEGORY_ID_minus_4"
:display-name "Min of ID + (2 × Average of Price × 3 × (Max of Category ID - 4))"}
;; user-specified names
@@ -94,7 +94,7 @@
{:display-name "User-specified Name"}
[:min {} (lib.tu/field-clause :venues :id)]
[:* {} 2 [:avg {} (lib.tu/field-clause :venues :price)]]]
- {:column-name "min_id_plus_2_times_avg_price"
+ {:column-name "min_ID_plus_2_times_avg_PRICE"
:display-name "User-specified Name"}))
;;; the following tests use raw legacy MBQL because they're direct ports of JavaScript tests from MLv1 and I wanted to
@@ -144,16 +144,16 @@
;; :sum
[:sum {} [:+ {} (lib.tu/field-clause :venues :price) 1]]
{:base_type :type/Integer
- :name "sum_price_plus_1"
+ :name "sum_PRICE_plus_1"
:display_name "Sum of Price + 1"}
;; options map
[:sum
{:name "sum_2", :display-name "My custom name", :base-type :type/BigInteger}
(lib.tu/field-clause :venues :price)]
- {:base_type :type/BigInteger
- :name "sum_2"
- :display_name "My custom name"}))
+ {:base_type :type/BigInteger
+ :name "sum_2"
+ :display_name "My custom name"}))
(deftest ^:parallel col-info-named-aggregation-test
(testing "col info for an `expression` aggregation w/ a named expression should work as expected"
@@ -173,12 +173,12 @@
(deftest ^:parallel aggregate-test
(let [q (lib/query-for-table-name meta/metadata-provider "VENUES")
result-query
- {:lib/type :mbql/query,
- :database (meta/id) ,
- :type :pipeline,
- :stages [{:lib/type :mbql.stage/mbql,
- :source-table (meta/id :venues) ,
- :lib/options {:lib/uuid string?},
+ {:lib/type :mbql/query
+ :database (meta/id)
+ :type :pipeline
+ :stages [{:lib/type :mbql.stage/mbql
+ :source-table (meta/id :venues)
+ :lib/options {:lib/uuid string?}
:aggregation [[:sum {:lib/uuid string?}
[:field
{:base-type :type/Integer, :lib/uuid string?}
@@ -248,7 +248,7 @@
(is (=? {:settings {:is_priceless true}
:lib/type :metadata/field
:base_type :type/Integer
- :name "sum_price"
+ :name "sum_PRICE"
:display_name "Sum of Price"
:lib/source :source/aggregations}
(lib.metadata.calculation/metadata query (first (lib/aggregations query -1))))))))
diff --git a/test/metabase/lib/expression_test.cljc b/test/metabase/lib/expression_test.cljc
index d30c00c02e4..53228f90229 100644
--- a/test/metabase/lib/expression_test.cljc
+++ b/test/metabase/lib/expression_test.cljc
@@ -119,7 +119,7 @@
(lib.tu/field-clause :checkins :date {:base-type :type/Date})
-1
:day]]
- (is (= "date_minus_1_day"
+ (is (= "DATE_minus_1_day"
(lib.metadata.calculation/column-name lib.tu/venues-query -1 clause)))
(is (= "Date - 1 day"
(lib.metadata.calculation/display-name lib.tu/venues-query -1 clause)))))
@@ -141,7 +141,7 @@
(deftest ^:parallel coalesce-names-test
(let [clause [:coalesce {} (lib.tu/field-clause :venues :name) "<Venue>"]]
- (is (= "name"
+ (is (= "NAME"
(lib.metadata.calculation/column-name lib.tu/venues-query -1 clause)))
(is (= "Name"
(lib.metadata.calculation/display-name lib.tu/venues-query -1 clause)))))
diff --git a/test/metabase/lib/field_test.cljc b/test/metabase/lib/field_test.cljc
index 596e7c86753..1d450d8faf7 100644
--- a/test/metabase/lib/field_test.cljc
+++ b/test/metabase/lib/field_test.cljc
@@ -1,6 +1,7 @@
(ns metabase.lib.field-test
(:require
[clojure.test :refer [deftest is testing]]
+ [medley.core :as m]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
@@ -151,6 +152,29 @@
(is (= "Date (year)"
(lib.metadata.calculation/display-name query -1 field))))))
+(deftest ^:parallel joined-field-column-name-test
+ (let [card {:dataset_query {:database (meta/id)
+ :type :query
+ :query {:source-table (meta/id :venues)
+ :joins [{:fields :all
+ :source-table (meta/id :categories)
+ :condition [:=
+ [:field (meta/id :venues :category-id) nil]
+ [:field (meta/id :categories :id) {:join-alias "Cat"}]]
+ :alias "Cat"}]}}}
+ query (lib/saved-question-query
+ meta/metadata-provider
+ card)]
+ (is (=? [{:lib/desired-column-alias "ID"}
+ {:lib/desired-column-alias "NAME"}
+ {:lib/desired-column-alias "CATEGORY_ID"}
+ {:lib/desired-column-alias "LATITUDE"}
+ {:lib/desired-column-alias "LONGITUDE"}
+ {:lib/desired-column-alias "PRICE"}
+ {:lib/desired-column-alias "Cat__ID"}
+ {:lib/desired-column-alias "Cat__NAME"}]
+ (lib.metadata.calculation/metadata query)))))
+
(deftest ^:parallel field-ref-type-of-test
(testing "Make sure we can calculate field ref type information correctly"
(let [clause [:field {:lib/uuid (str (random-uuid))} (meta/id :venues :id)]]
@@ -159,6 +183,17 @@
(is (= :type/BigInteger
(lib.metadata.calculation/type-of lib.tu/venues-query clause))))))
+(deftest ^:parallel implicitly-joinable-field-display-name-test
+ (testing "Should be able to calculate a display name for an implicitly joinable Field"
+ (let [query (lib/query-for-table-name meta/metadata-provider "VENUES")
+ categories-name (m/find-first #(= (:id %) (meta/id :categories :name))
+ (lib/orderable-columns query))]
+ (is (= "Categories → Name"
+ (lib/display-name query categories-name)))
+ (let [query' (lib/order-by query categories-name)]
+ (is (= "Venues, Sorted by Categories → Name ascending"
+ (lib/describe-query query')))))))
+
(deftest ^:parallel source-card-table-display-info-test
(let [query (assoc lib.tu/venues-query :lib/metadata lib.tu/metadata-provider-with-card)
field (lib.metadata.calculation/metadata query (assoc (lib.metadata/field query (meta/id :venues :name))
diff --git a/test/metabase/lib/metadata/jvm_test.clj b/test/metabase/lib/metadata/jvm_test.clj
new file mode 100644
index 00000000000..fc73d59e228
--- /dev/null
+++ b/test/metabase/lib/metadata/jvm_test.clj
@@ -0,0 +1,30 @@
+(ns metabase.lib.metadata.jvm-test
+ (:require
+ [clojure.test :refer :all]
+ [metabase.lib.core :as lib]
+ [metabase.lib.metadata.calculation :as lib.metadata.calculation]
+ [metabase.lib.metadata.jvm :as sut]
+ [metabase.test :as mt]))
+
+(deftest ^:parallel saved-question-metadata-test
+ (let [card {:dataset_query {:database (mt/id)
+ :type :query
+ :query {:source-table (mt/id :venues)
+ :joins [{:fields :all
+ :source-table (mt/id :categories)
+ :condition [:=
+ [:field (mt/id :venues :category_id) nil]
+ [:field (mt/id :categories :id) {:join-alias "Cat"}]]
+ :alias "Cat"}]}}}
+ query (lib/saved-question-query
+ (metabase.lib.metadata.jvm/application-database-metadata-provider (mt/id))
+ card)]
+ (is (=? [{:lib/desired-column-alias "ID"}
+ {:lib/desired-column-alias "NAME"}
+ {:lib/desired-column-alias "CATEGORY_ID"}
+ {:lib/desired-column-alias "LATITUDE"}
+ {:lib/desired-column-alias "LONGITUDE"}
+ {:lib/desired-column-alias "PRICE"}
+ {:lib/desired-column-alias "Cat__ID"}
+ {:lib/desired-column-alias "Cat__NAME"}]
+ (lib.metadata.calculation/metadata query)))))
diff --git a/test/metabase/lib/order_by_test.cljc b/test/metabase/lib/order_by_test.cljc
index 21c22685a90..6fd10546608 100644
--- a/test/metabase/lib/order_by_test.cljc
+++ b/test/metabase/lib/order_by_test.cljc
@@ -148,12 +148,12 @@
:base_type :type/Integer}
{:lib/type :metadata/field
:base_type :type/Integer
- :name "sum_price"
+ :name "sum_PRICE"
:display_name "Sum of Price"
:lib/source :source/aggregations}
{:lib/type :metadata/field
:base_type :type/Float
- :name "avg_price_plus_1"
+ :name "avg_PRICE_plus_1"
:display_name "Average of Price + 1"
:lib/source :source/aggregations}]
(lib/orderable-columns query)))))))
@@ -174,53 +174,69 @@
(deftest ^:parallel orderable-columns-test
(let [query (lib/query-for-table-name meta/metadata-provider "VENUES")]
(testing (lib.util/format "Query =\n%s" (u/pprint-to-str query))
- (is (=? [{:lib/type :metadata/field
- :name "ID"
- :display_name "ID"
- :id (meta/id :venues :id)
- :table_id (meta/id :venues)
- :base_type :type/BigInteger}
- {:lib/type :metadata/field
- :name "NAME"
- :display_name "Name"
- :id (meta/id :venues :name)
- :table_id (meta/id :venues)
- :base_type :type/Text}
- {:lib/type :metadata/field
- :name "CATEGORY_ID"
- :display_name "Category ID"
- :id (meta/id :venues :category-id)
- :table_id (meta/id :venues)}
- {:lib/type :metadata/field
- :name "LATITUDE"
- :display_name "Latitude"
- :id (meta/id :venues :latitude)
- :table_id (meta/id :venues)
- :base_type :type/Float}
- {:lib/type :metadata/field
- :name "LONGITUDE"
- :display_name "Longitude"
- :id (meta/id :venues :longitude)
- :table_id (meta/id :venues)
- :base_type :type/Float}
- {:lib/type :metadata/field
- :name "PRICE"
- :display_name "Price"
- :id (meta/id :venues :price)
- :table_id (meta/id :venues)
- :base_type :type/Integer}
- {:lib/type :metadata/field
- :name "ID"
- :display_name "ID"
- :id (meta/id :categories :id)
- :table_id (meta/id :categories)
- :base_type :type/BigInteger}
- {:lib/type :metadata/field
- :name "NAME"
- :display_name "Name"
- :id (meta/id :categories :name)
- :table_id (meta/id :categories)
- :base_type :type/Text}]
+ (is (=? [{:lib/type :metadata/field
+ :name "ID"
+ :display_name "ID"
+ :id (meta/id :venues :id)
+ :table_id (meta/id :venues)
+ :base_type :type/BigInteger
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "ID"}
+ {:lib/type :metadata/field
+ :name "NAME"
+ :display_name "Name"
+ :id (meta/id :venues :name)
+ :table_id (meta/id :venues)
+ :base_type :type/Text
+ :lib/source-column-alias "NAME"
+ :lib/desired-column-alias "NAME"}
+ {:lib/type :metadata/field
+ :name "CATEGORY_ID"
+ :display_name "Category ID"
+ :id (meta/id :venues :category-id)
+ :table_id (meta/id :venues)
+ :lib/source-column-alias "CATEGORY_ID"
+ :lib/desired-column-alias "CATEGORY_ID"}
+ {:lib/type :metadata/field
+ :name "LATITUDE"
+ :display_name "Latitude"
+ :id (meta/id :venues :latitude)
+ :table_id (meta/id :venues)
+ :base_type :type/Float
+ :lib/source-column-alias "LATITUDE"
+ :lib/desired-column-alias "LATITUDE"}
+ {:lib/type :metadata/field
+ :name "LONGITUDE"
+ :display_name "Longitude"
+ :id (meta/id :venues :longitude)
+ :table_id (meta/id :venues)
+ :base_type :type/Float
+ :lib/source-column-alias "LONGITUDE"
+ :lib/desired-column-alias "LONGITUDE"}
+ {:lib/type :metadata/field
+ :name "PRICE"
+ :display_name "Price"
+ :id (meta/id :venues :price)
+ :table_id (meta/id :venues)
+ :base_type :type/Integer
+ :lib/source-column-alias "PRICE"
+ :lib/desired-column-alias "PRICE"}
+ {:lib/type :metadata/field
+ :name "ID"
+ :display_name "ID"
+ :id (meta/id :categories :id)
+ :table_id (meta/id :categories)
+ :base_type :type/BigInteger
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "CATEGORIES__via__CATEGORY_ID__ID"}
+ {:lib/type :metadata/field
+ :name "NAME"
+ :display_name "Name"
+ :id (meta/id :categories :name)
+ :table_id (meta/id :categories)
+ :base_type :type/Text
+ :lib/source-column-alias "NAME"
+ :lib/desired-column-alias "CATEGORIES__via__CATEGORY_ID__NAME"}]
(lib/orderable-columns query))))))
(deftest ^:parallel orderable-expressions-test
@@ -336,32 +352,83 @@
[:field {:lib/uuid string? :base-type :type/Text} (meta/id :venues :name)]]]
(lib/order-bys query'))))))))
+(deftest ^:parallel orderable-columns-with-join-test
+ (is (=? [{:name "ID"
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "ID"
+ :lib/source :source/table-defaults}
+ {:name "NAME"
+ :lib/source-column-alias "NAME"
+ :lib/desired-column-alias "NAME"
+ :lib/source :source/table-defaults}
+ {:name "CATEGORY_ID"
+ :lib/source-column-alias "CATEGORY_ID"
+ :lib/desired-column-alias "CATEGORY_ID"
+ :lib/source :source/table-defaults}
+ {:name "LATITUDE"
+ :lib/source-column-alias "LATITUDE"
+ :lib/desired-column-alias "LATITUDE"
+ :lib/source :source/table-defaults}
+ {:name "LONGITUDE"
+ :lib/source-column-alias "LONGITUDE"
+ :lib/desired-column-alias "LONGITUDE"
+ :lib/source :source/table-defaults}
+ {:name "PRICE"
+ :lib/source-column-alias "PRICE"
+ :lib/desired-column-alias "PRICE"
+ :lib/source :source/table-defaults}
+ {:name "ID"
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "Cat__ID"
+ :lib/source :source/joins}
+ {:name "NAME"
+ :lib/source-column-alias "NAME"
+ :lib/desired-column-alias "Cat__NAME"
+ :lib/source :source/joins}]
+ (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/join (-> (lib/join-clause
+ (meta/table-metadata :categories)
+ (lib/=
+ (lib/field "VENUES" "CATEGORY_ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Cat")))
+ (lib/with-join-alias "Cat")
+ (lib/with-join-fields :all)))
+ (lib/fields [(lib/field "VENUES" "ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Cat")])
+ (lib/orderable-columns)))))
+
(deftest ^:parallel order-bys-with-duplicate-column-names-test
(testing "Order by stuff should work with two different columns named ID (#29702)"
- (is (=? [{:id (meta/id :venues :id)
- :name "ID"
- :lib/source :source/previous-stage
- :lib/type :metadata/field
- :base_type :type/BigInteger
- :effective_type :type/BigInteger
- :display_name "ID"
- :table_id (meta/id :venues)}
- {:id (meta/id :categories :id)
- :name "ID_2"
- :lib/source :source/previous-stage
- :lib/type :metadata/field
- :base_type :type/BigInteger
- :effective_type :type/BigInteger
- :display_name "ID"
- :table_id (meta/id :categories)}]
+ (is (=? [{:id (meta/id :venues :id)
+ :name "ID"
+ :lib/source :source/previous-stage
+ :lib/type :metadata/field
+ :base_type :type/BigInteger
+ :effective_type :type/BigInteger
+ :display_name "ID"
+ :table_id (meta/id :venues)
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "ID"}
+ {:id (meta/id :categories :id)
+ :name "ID"
+ :lib/source :source/previous-stage
+ :lib/type :metadata/field
+ :base_type :type/BigInteger
+ :effective_type :type/BigInteger
+ :display_name "Categories → ID"
+ :table_id (meta/id :categories)
+ :lib/source-column-alias "Cat__ID"
+ :lib/desired-column-alias "Cat__ID"}]
(-> (lib/query-for-table-name meta/metadata-provider "VENUES")
(lib/join (-> (lib/join-clause
(meta/table-metadata :categories)
(lib/=
(lib/field "VENUES" "CATEGORY_ID")
- (lib/field "CATEGORIES" "ID")))
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Cat")))
+ (lib/with-join-alias "Cat")
(lib/with-join-fields :all)))
- (lib/fields [(lib/field "VENUES" "ID") (lib/field "CATEGORIES" "ID")])
+ (lib/fields [(lib/field "VENUES" "ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Cat")])
(lib/append-stage)
(lib/orderable-columns))))))
@@ -463,7 +530,7 @@
{:source-field (meta/id :venues :category-id)}
(meta/id :categories :name)]]]}]}
query))
- (is (= "Venues, Sorted by Name ascending"
+ (is (= "Venues, Sorted by Categories → Name ascending"
(lib/describe-query query)))
(is (=? [{:display_name "ID", :lib/source :source/table-defaults}
{:display_name "Name", :lib/source :source/table-defaults}
@@ -504,17 +571,16 @@
(let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
(lib/expression "expr" (lib/absolute-datetime "2020" :month))
(lib/fields [(lib/field "VENUES" "ID")]))]
- (is (= [{:id (meta/id :venues :id), :name "ID", :display_name "ID", :lib/source :source/table-defaults}
- {:id (meta/id :venues :name), :name "NAME", :display_name "Name", :lib/source :source/table-defaults}
- {:id (meta/id :venues :category-id), :name "CATEGORY_ID", :display_name "Category ID", :lib/source :source/table-defaults}
- {:id (meta/id :venues :latitude), :name "LATITUDE", :display_name "Latitude", :lib/source :source/table-defaults}
- {:id (meta/id :venues :longitude), :name "LONGITUDE", :display_name "Longitude", :lib/source :source/table-defaults}
- {:id (meta/id :venues :price), :name "PRICE", :display_name "Price", :lib/source :source/table-defaults}
- {:name "expr", :display_name "expr", :lib/source :source/expressions}
- {:id (meta/id :categories :id), :name "ID", :display_name "ID", :lib/source :source/implicitly-joinable}
- {:id (meta/id :categories :name), :name "NAME", :display_name "Name", :lib/source :source/implicitly-joinable}]
- (map #(select-keys % [:id :name :display_name :lib/source])
- (lib/orderable-columns query))))
+ (is (=? [{:id (meta/id :venues :id), :name "ID", :display_name "ID", :lib/source :source/table-defaults}
+ {:id (meta/id :venues :name), :name "NAME", :display_name "Name", :lib/source :source/table-defaults}
+ {:id (meta/id :venues :category-id), :name "CATEGORY_ID", :display_name "Category ID", :lib/source :source/table-defaults}
+ {:id (meta/id :venues :latitude), :name "LATITUDE", :display_name "Latitude", :lib/source :source/table-defaults}
+ {:id (meta/id :venues :longitude), :name "LONGITUDE", :display_name "Longitude", :lib/source :source/table-defaults}
+ {:id (meta/id :venues :price), :name "PRICE", :display_name "Price", :lib/source :source/table-defaults}
+ {:name "expr", :display_name "expr", :lib/source :source/expressions}
+ {:id (meta/id :categories :id), :name "ID", :display_name "ID", :lib/source :source/implicitly-joinable}
+ {:id (meta/id :categories :name), :name "NAME", :display_name "Name", :lib/source :source/implicitly-joinable}]
+ (lib/orderable-columns query)))
(let [expr (m/find-first #(= (:name %) "expr") (lib/orderable-columns query))]
(is (=? {:lib/type :metadata/field
:lib/source :source/expressions
diff --git a/test/metabase/lib/stage_test.cljc b/test/metabase/lib/stage_test.cljc
index 61a0edfd5f5..b475fa578d2 100644
--- a/test/metabase/lib/stage_test.cljc
+++ b/test/metabase/lib/stage_test.cljc
@@ -36,18 +36,22 @@
(let [query (lib.tu/venues-query-with-last-stage
{:aggregation [[:*
{}
- 0.9
+ 0.8
[:avg {} (lib.tu/field-clause :venues :price)]]
[:*
{}
0.8
[:avg {} (lib.tu/field-clause :venues :price)]]]})]
- (is (=? [{:base_type :type/Float
- :name "0_9_times_avg_price"
- :display_name "0.9 × Average of Price"}
- {:base_type :type/Float
- :name "0_8_times_avg_price"
- :display_name "0.8 × Average of Price"}]
+ (is (=? [{:base_type :type/Float
+ :name "0_8_times_avg_PRICE"
+ :display_name "0.8 × Average of Price"
+ :lib/source-column-alias "0_8_times_avg_PRICE"
+ :lib/desired-column-alias "0_8_times_avg_PRICE"}
+ {:base_type :type/Float
+ :name "0_8_times_avg_PRICE"
+ :display_name "0.8 × Average of Price"
+ :lib/source-column-alias "0_8_times_avg_PRICE"
+ :lib/desired-column-alias "0_8_times_avg_PRICE_2"}]
(lib.metadata.calculation/metadata query -1 query)))))))
(deftest ^:parallel stage-display-name-card-source-query
@@ -81,9 +85,9 @@
:stages [{:lib/type :mbql.stage/mbql
:lib/options {:lib/uuid (str (random-uuid))}
:source-table "card__1"}]}]
- (is (= (for [col (:columns meta/results-metadata)]
- (assoc col :lib/source :source/card))
- (lib.metadata.calculation/metadata query -1 query)))))))
+ (is (=? (for [col (:columns meta/results-metadata)]
+ (assoc col :lib/source :source/card))
+ (lib.metadata.calculation/metadata query -1 query)))))))
(deftest ^:parallel adding-and-removing-stages
(let [query (lib/query-for-table-name meta/metadata-provider "VENUES")
@@ -147,16 +151,20 @@
{:id (meta/id :venues :price), :name "PRICE", :lib/source :source/table-defaults}
{:name "ID + 1", :lib/source :source/expressions}
{:name "ID + 2", :lib/source :source/expressions}
- {:id (meta/id :categories :id)
- :name "ID_2"
- :lib/source :source/joins
- :source_alias "Cat"
- :display_name "Categories → ID"}
- {:id (meta/id :categories :name)
- :name "NAME_2"
- :lib/source :source/joins
- :source_alias "Cat"
- :display_name "Categories → Name"}]
+ {:id (meta/id :categories :id)
+ :name "ID"
+ :lib/source :source/joins
+ :source_alias "Cat"
+ :display_name "Categories → ID"
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "Cat__ID"}
+ {:id (meta/id :categories :name)
+ :name "NAME"
+ :lib/source :source/joins
+ :source_alias "Cat"
+ :display_name "Categories → Name"
+ :lib/source-column-alias "NAME"
+ :lib/desired-column-alias "Cat__NAME"}]
(lib.metadata.calculation/metadata query))))))
(deftest ^:parallel metadata-with-fields-only-include-expressions-in-fields-test
@@ -185,3 +193,36 @@
(let [[id-plus-1] (lib.metadata.calculation/metadata query')]
(is (=? [:expression {:base-type :type/Integer, :effective-type :type/Integer} "ID + 1"]
(lib/ref id-plus-1))))))))))
+
+(deftest ^:parallel joins-source-and-desired-aliases-test
+ (let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+ (lib/join (-> (lib/join-clause
+ (meta/table-metadata :categories)
+ (lib/=
+ (lib/field "VENUES" "CATEGORY_ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Cat")))
+ (lib/with-join-alias "Cat")
+ (lib/with-join-fields :all)))
+ (lib/fields [(lib/field "VENUES" "ID")
+ (lib/with-join-alias (lib/field "CATEGORIES" "ID") "Cat")]))]
+ (is (=? [{:name "ID"
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "ID"
+ :lib/source :source/fields}
+ {:name "ID"
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "Cat__ID"
+ :metabase.lib.field/join-alias "Cat"
+ :lib/source :source/fields}]
+ (lib.metadata.calculation/metadata query)))
+ (testing "Introduce a new stage"
+ (let [query' (lib/append-stage query)]
+ (is (=? [{:name "ID"
+ :lib/source-column-alias "ID"
+ :lib/desired-column-alias "ID"
+ :lib/source :source/previous-stage}
+ {:name "ID"
+ :lib/source-column-alias "Cat__ID"
+ :lib/desired-column-alias "Cat__ID"
+ :lib/source :source/previous-stage}]
+ (lib.metadata.calculation/metadata query')))))))
diff --git a/test/metabase/lib/util_test.cljc b/test/metabase/lib/util_test.cljc
index a1554b85d51..e70d37a08f2 100644
--- a/test/metabase/lib/util_test.cljc
+++ b/test/metabase/lib/util_test.cljc
@@ -1,5 +1,6 @@
(ns metabase.lib.util-test
(:require
+ [clojure.string :as str]
[clojure.test :refer [are deftest is testing]]
[metabase.lib.test-metadata :as meta]
[metabase.lib.util :as lib.util]
@@ -216,3 +217,92 @@
["a" "b"] "a and b"
["a" "b" "c"] "a, b, and c"
["a" "b" "c" "d"] "a, b, c, and d"))
+
+(deftest ^:parallel crc32-checksum-test
+ (are [s checksum] (= checksum
+ (#'lib.util/crc32-checksum s))
+ "YMRZFRTHUBOUZHPTZGPD" "2694651f"
+ "MEBRXTJEPWOJJXVZIPDA" "048132cb"
+ "UIOJOTPGUIROVRJYAFPO" "0085cacb"
+ "UCVEWTGNBDANGMZPGNQC" "000e32a0"
+ "ZAFVKSVXQKJNGANBQZMX" "0000d5b8"
+ "NCTFDMQNUEQLJUMAGSYG" "000000ea"
+ "YHQJXDIXGGQTSARGOQZZ" "000000c1"
+ "0601246074" "00000001"
+ "2915035893" "00000000"))
+
+(deftest ^:parallel truncate-string-to-byte-count-test
+ (letfn [(truncate-string-to-byte-count [s byte-length]
+ (let [truncated (#'lib.util/truncate-string-to-byte-count s byte-length)]
+ (is (<= (#'lib.util/string-byte-count truncated) byte-length))
+ (is (str/starts-with? s truncated))
+ truncated))]
+ (doseq [[s max-length->expected] {"12345"
+ {1 "1"
+ 2 "12"
+ 3 "123"
+ 4 "1234"
+ 5 "12345"
+ 6 "12345"
+ 10 "12345"}
+
+ "가나다ë¼"
+ {1 ""
+ 2 ""
+ 3 "ê°€"
+ 4 "ê°€"
+ 5 "ê°€"
+ 6 "가나"
+ 7 "가나"
+ 8 "가나"
+ 9 "가나다"
+ 10 "가나다"
+ 11 "가나다"
+ 12 "가나다ë¼"
+ 13 "가나다ë¼"
+ 15 "가나다ë¼"
+ 20 "가나다ë¼"}}
+ [max-length expected] max-length->expected]
+ (testing (pr-str (list `lib.util/truncate-string-to-byte-count s max-length))
+ (is (= expected
+ (truncate-string-to-byte-count s max-length)))))))
+
+(deftest ^:parallel truncate-alias-test
+ (letfn [(truncate-alias [s max-bytes]
+ (let [truncated (lib.util/truncate-alias s max-bytes)]
+ (is (<= (#'lib.util/string-byte-count truncated) max-bytes))
+ truncated))]
+ (doseq [[s max-bytes->expected] { ;; 20-character plain ASCII string
+ "01234567890123456789"
+ {12 "012_fc89bad5"
+ 15 "012345_fc89bad5"
+ 20 "01234567890123456789"}
+
+ ;; two strings that only differ after the point they get truncated
+ "0123456789abcde" {12 "012_1629bb92"}
+ "0123456789abcdE" {12 "012_2d479b5a"}
+
+ ;; Unicode string: 14 characters, 42 bytes
+ "가나다ë¼ë§ˆë°”사아ìžì°¨ì¹´íƒ€íŒŒí•˜"
+ {12 "ê°€_b9c95392"
+ 13 "ê°€_b9c95392"
+ 14 "ê°€_b9c95392"
+ 15 "가나_b9c95392"
+ 20 "가나다_b9c95392"
+ 30 "가나다ë¼ë§ˆë°”사_b9c95392"
+ 40 "가나다ë¼ë§ˆë°”사아ìžì°¨_b9c95392"
+ 50 "가나다ë¼ë§ˆë°”사아ìžì°¨ì¹´íƒ€íŒŒí•˜"}
+
+ ;; Mixed string: 17 characters, 33 bytes
+ "aê°€b나c다dë¼e마fë°”g사hì•„i"
+ {12 "a_99a0fe0c"
+ 13 "aê°€_99a0fe0c"
+ 14 "aê°€b_99a0fe0c"
+ 15 "aê°€b_99a0fe0c"
+ 20 "a가b나c_99a0fe0c"
+ 30 "aê°€b나c다dë¼e마f_99a0fe0c"
+ 40 "aê°€b나c다dë¼e마fë°”g사hì•„i"}}
+ [max-bytes expected] max-bytes->expected]
+ (testing (pr-str (list `lib.util/truncate-alias s max-bytes))
+ (is (= expected
+ (truncate-alias s max-bytes)))))))
diff --git a/test/metabase/models/metric_test.clj b/test/metabase/models/metric_test.clj
index b4c37b26869..13cf23a6288 100644
--- a/test/metabase/models/metric_test.clj
+++ b/test/metabase/models/metric_test.clj
@@ -144,7 +144,7 @@
:filter [:and
[:= $price 4]
[:segment segment-id]]}))}]
- (is (= "Venues, Sum of Name, Filtered by Price equals 4 and Checkins with ID = 1"
+ (is (= "Venues, Sum of Categories → Name, Filtered by Price equals 4 and Checkins with ID = 1"
(:definition_description (t2/hydrate metric :definition_description))))))
(deftest definition-description-missing-source-table-test
@@ -154,7 +154,7 @@
:definition (mt/$ids venues
{:aggregation [[:sum $category_id->categories.name]]
:filter [:= $price 4]})}]
- (is (= "Venues, Sum of Name, Filtered by Price equals 4"
+ (is (= "Venues, Sum of Categories → Name, Filtered by Price equals 4"
(:definition_description (t2/hydrate metric :definition_description)))))))
(deftest definition-description-invalid-query-test
diff --git a/test/metabase/models/segment_test.clj b/test/metabase/models/segment_test.clj
index b9eeae877cb..2de2445c9e4 100644
--- a/test/metabase/models/segment_test.clj
+++ b/test/metabase/models/segment_test.clj
@@ -131,7 +131,7 @@
[:and
[:= $price 4]
[:= $category_id->categories.name "BBQ"]]}))}]
- (is (= "Filtered by Price equals 4 and Name equals \"BBQ\""
+ (is (= "Filtered by Price equals 4 and Categories → Name equals \"BBQ\""
(:definition_description (t2/hydrate segment :definition_description))))
(testing "Segments that reference other Segments (inception)"
(t2.with-temp/with-temp [Segment segment-2 {:name "Segment 2"
diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/metabase/query_processor_test/explicit_joins_test.clj
index 166598c4c67..0b91aecfc95 100644
--- a/test/metabase/query_processor_test/explicit_joins_test.clj
+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
@@ -822,7 +822,7 @@
(str/join (for [i (range length)]
(nth charset (mod i (count charset))))))
-(deftest very-long-join-name-test
+(deftest ^:parallel very-long-join-name-test
(mt/test-drivers (mt/normal-drivers-with-feature :left-join)
(testing "Drivers should work correctly even if joins have REALLLLLLY long names (#15978)"
(doseq [[charset-name charset] charsets
diff --git a/yarn.lock b/yarn.lock
index ab03bda00e5..83b5c06bd14 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -9354,6 +9354,11 @@ cpy@^8.1.2:
p-filter "^2.1.0"
p-map "^3.0.0"
+crc-32@^1.2.2:
+ version "1.2.2"
+ resolved "https://registry.yarnpkg.com/crc-32/-/crc-32-1.2.2.tgz#3cad35a934b8bf71f25ca524b6da51fb7eace2ff"
+ integrity sha512-ROmzCKrTnOwybPcJApAA6WBWij23HVfGVNKqqrZpuyZOHqK2CwHSvpGuyt/UNNvaIjEd8X5IFGp4Mh+Ie1IHJQ==
+
crc-32@~1.2.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/crc-32/-/crc-32-1.2.0.tgz#cb2db6e29b88508e32d9dd0ec1693e7b41a18208"
--
GitLab