Skip to content
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

Relative date filters don't work with custom date columns #33528

Closed
ranquild opened this issue Aug 28, 2023 · 2 comments · Fixed by #38874
Closed

Relative date filters don't work with custom date columns #33528

ranquild opened this issue Aug 28, 2023 · 2 comments · Fixed by #38874
Assignees
Labels
.Backend Priority:P2 Average run of the mill bug Querying/Processor .Team/QueryProcessor :hammer_and_wrench: Type:Bug Product defects .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2
Milestone

Comments

@ranquild
Copy link
Contributor

ranquild commented Aug 28, 2023

Describe the bug

When using a relative date filter with a custom date column the query always returns "No results" despite data being present for the selected range.

To Reproduce

  1. New -> Question -> Orders
  2. Add custom column = [Created At], with name DateColumn
  3. Filter -> DateColumn -> Relative dates -> Current -> Year
  4. Visualize

Actual result:

  • No results found
Screenshot 2023-08-28 at 14 57 51

Expected result:

  • Rows are returned for the current year. Compare with filtering by Created At directly.
Screenshot 2023-08-28 at 14 57 57

Expected behavior

No response

Logs

No response

Information about your Metabase installation

0.47.0

Severity

P2

Additional context

No response

@ranquild ranquild added Type:Bug Product defects .Needs Triage Priority:P2 Average run of the mill bug .Team/QueryProcessor :hammer_and_wrench: Querying/Processor .Backend and removed .Needs Triage labels Aug 28, 2023
@kulyk kulyk added the .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2 label Aug 28, 2023
@kamilmielnik kamilmielnik self-assigned this Nov 8, 2023
@kamilmielnik
Copy link
Contributor

Reproducible in 0.47.0.
Reproducible in mlv2-fe-filters/chill-mode (#34851 @ e546daa) and in master (d35c779).

@kamilmielnik kamilmielnik removed their assignment Nov 8, 2023
@bshepherdson
Copy link
Contributor

I can reproduce this on master today too.

Bad SQL with custom column:

SELECT
  "source"."ID" AS "ID",
  "source"."USER_ID" AS "USER_ID",
  "source"."PRODUCT_ID" AS "PRODUCT_ID",
  "source"."SUBTOTAL" AS "SUBTOTAL",
  "source"."TAX" AS "TAX",
  "source"."TOTAL" AS "TOTAL",
  "source"."DISCOUNT" AS "DISCOUNT",
  "source"."CREATED_AT" AS "CREATED_AT",
  "source"."QUANTITY" AS "QUANTITY",
  "source"."DateColumn" AS "DateColumn"
FROM (
  SELECT
    "PUBLIC"."ORDERS"."ID" AS "ID",
    "PUBLIC"."ORDERS"."USER_ID" AS "USER_ID",
    "PUBLIC"."ORDERS"."PRODUCT_ID" AS "PRODUCT_ID",
    "PUBLIC"."ORDERS"."SUBTOTAL" AS "SUBTOTAL",
    "PUBLIC"."ORDERS"."TAX" AS "TAX",
    "PUBLIC"."ORDERS"."TOTAL" AS "TOTAL",
    "PUBLIC"."ORDERS"."DISCOUNT" AS "DISCOUNT",
    "PUBLIC"."ORDERS"."CREATED_AT" AS "CREATED_AT",
    "PUBLIC"."ORDERS"."QUANTITY" AS "QUANTITY",
    "PUBLIC"."ORDERS"."CREATED_AT" AS "DateColumn"
  FROM "PUBLIC"."ORDERS") AS "source"
WHERE
  "source"."DateColumn" = DATE_TRUNC('year', NOW())
LIMIT 2000

which looks plausible at a glance but doesn't return any data. The correct query (on Created At directly rather than the custom column) generates this filter instead:

WHERE (
  "PUBLIC"."ORDERS"."CREATED_AT" >= DATE_TRUNC('year', NOW()))
AND 
  "PUBLIC"."ORDERS"."CREATED_AT" < DATE_TRUNC('year', DATEADD('year', CAST(1 AS long), CAST(NOW() AS datetime)))

I guess the custom column's type isn't being correctly established. This may Just Work with MLv2-powered custom column types in the QP, or it might require a deliberate fix. (Maybe instead the resulting type of just a year number is being computed for source.DateColumn even though it's the entire date and hasn't been truncated yet.)

@snoe snoe self-assigned this Feb 15, 2024
snoe added a commit that referenced this issue Feb 16, 2024
Fixes #33528

Turns out that `mbql/normalize` and `lib/normalize` behaved slightly
different and `[:expression "abc" {...}]` refs would drop their opts in
the former path. In order to properly query against binned datetimes
it's important that expression ref does not lose its type or else the
optimizer will not see that `time-interval` needs to convert to a
`between` rather than an `=`.
snoe added a commit that referenced this issue Feb 20, 2024
* Fixes filters against datetime binned expressions

Fixes #33528

Turns out that `mbql/normalize` and `lib/normalize` behaved slightly
different and `[:expression "abc" {...}]` refs would drop their opts in
the former path. In order to properly query against binned datetimes
it's important that expression ref does not lose its type or else the
optimizer will not see that `time-interval` needs to convert to a
`between` rather than an `=`.

* Fix test

* Only keep specific keys on expression opts for these expression filters

* Don't run checkin dataset on snowflake or athena
github-actions bot pushed a commit that referenced this issue Feb 20, 2024
* Fixes filters against datetime binned expressions

Fixes #33528

Turns out that `mbql/normalize` and `lib/normalize` behaved slightly
different and `[:expression "abc" {...}]` refs would drop their opts in
the former path. In order to properly query against binned datetimes
it's important that expression ref does not lose its type or else the
optimizer will not see that `time-interval` needs to convert to a
`between` rather than an `=`.

* Fix test

* Only keep specific keys on expression opts for these expression filters

* Don't run checkin dataset on snowflake or athena
@snoe snoe added this to the 0.49 milestone Feb 20, 2024
metabase-bot bot added a commit that referenced this issue Feb 27, 2024
* Fixes filters against datetime binned expressions

Fixes #33528

Turns out that `mbql/normalize` and `lib/normalize` behaved slightly
different and `[:expression "abc" {...}]` refs would drop their opts in
the former path. In order to properly query against binned datetimes
it's important that expression ref does not lose its type or else the
optimizer will not see that `time-interval` needs to convert to a
`between` rather than an `=`.

* Fix test

* Only keep specific keys on expression opts for these expression filters

* Don't run checkin dataset on snowflake or athena

Co-authored-by: Case Nelson <case@metabase.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Backend Priority:P2 Average run of the mill bug Querying/Processor .Team/QueryProcessor :hammer_and_wrench: Type:Bug Product defects .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants