-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom Expression case
is using wrong field reference when nested query
#17512
Comments
Working on getting #16656 merged in to make this sort of thing easy to fix. |
I'm trying to repro this and not able to get the same error message:
I can reproduce with the following: I'm asking because I'm working on #18513. I've solved that issue and this one feels like it might be similar. The problem there is that when doing type inference on the expression, it returns the field's entire information. For coalesce, it just returns the type inference on the second field. This includes a field-id and this is why it emits a reference to that field rather than to the expression's name. I suspect the error is the same here but i'm unable to reproduce it at the moment. ;; middleware/annotate.clj
(defn infer-expression-type
"Infer base-type/semantic-type information about an `expression` clause."
[expression]
(cond
(string? expression)
{:base_type :type/Text}
(number? expression)
{:base_type :type/Number}
(mbql.u/is-clause? :field expression)
(col-info-for-field-clause {} expression)
(mbql.u/is-clause? :coalesce expression)
(infer-expression-type (second expression)) ;; this will return a field's information including id and name. But the id is what is untenable
(mbql.u/is-clause? :length expression)
{:base_type :type/BigInteger}
(mbql.u/is-clause? :case expression)
(let [[_ clauses] expression]
(some
(fn [[_ expression]]
;; get the first non-nil val
(when (and (not= expression nil)
(or (not (mbql.u/is-clause? :value expression))
(let [[_ value] expression]
(not= value nil))))
(infer-expression-type expression)))
clauses))
(mbql.u/datetime-arithmetics? expression)
{:base_type :type/DateTime}
(mbql.u/is-clause? mbql.s/string-expressions expression)
{:base_type :type/Text}
(mbql.u/is-clause? mbql.s/arithmetic-expressions expression)
{:base_type :type/Float}
:else
{:base_type :type/*})) Changing that coalesce clause to (mbql.u/is-clause? :coalesce expression)
(select-keys (infer-expression-type (second expression))
[:base_type :effective_type :coercion_strategy :semantic_type]) fixes this defect by ensuring it only takes the type information of the target field. The id is so "poisonous" because This is why i suspect this is the same issue. A field id propagates from the case type inference leading to emitting selecting that field from the nested query. |
@dpsutton You cannot use those functions on Custom Column. You'll have to use it for Custom Expression. The frontend doesn't correctly block functions that cannot be used, but that's a known issue. |
Describe the bug
Custom Expression
case
is using wrong field reference when nested queryRegression since 0.40.0
To Reproduce
Distinct(case([Discount] > 0, [Subtotal], [Total]))
as "CE", grouped by CreatedAt:Month1 + 1
as "CC" (to trigger nested query)Column "source.SUBTOTAL" not found;
, because it's incorrectly referencing the field instead of the expression.Generated SQL query
Full stacktrace
Information about your Metabase Installation:
Tested 0.39.4 thru 0.40.2 - regression since 0.40.0
Additional context
Slightly similar to #14859, which was fixed in 0.38.0
The text was updated successfully, but these errors were encountered: