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

current week returns result, but "past month" returns nothing #164

Closed
taylorchu opened this issue May 20, 2023 · 10 comments
Closed

current week returns result, but "past month" returns nothing #164

taylorchu opened this issue May 20, 2023 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@taylorchu
Copy link

taylorchu commented May 20, 2023

Describe the bug

current week returns result, but "past 1 month" returns nothing.
it seems the result is inconsistent across timestamp filter.

Steps to reproduce

Screenshot 2023-05-19 at 6 39 41 PM Screenshot 2023-05-19 at 6 38 42 PM
  1. choose "timestamp" to filter to "this" week. it shows N rows
  2. choose "timestamp" to filter last N months including this week shows no rows

Expected behaviour

filtering by time works.

Error log

Configuration

Environment

  • metabase-clickhouse-driver version: 1.1.5
  • Metabase version: 0.46.3

ClickHouse server

  • ClickHouse Server version: 23.4
@taylorchu taylorchu added the bug Something isn't working label May 20, 2023
@taylorchu taylorchu changed the title current week returns result, but "past 2 weeks" returns nothing current week returns result, but "past month" returns nothing May 20, 2023
@taylorchu
Copy link
Author

taylorchu commented May 20, 2023

also

  1. current week works but current months return nothing
  2. it seems to correlate to time units.
    minute, hour, and week work, but day, month, quarter and year do not work.

@taylorchu
Copy link
Author

@slvrtrn could you take a look? it seems like you have a pr, but not sure if it is related.

@slvrtrn
Copy link
Collaborator

slvrtrn commented May 22, 2023

@taylorchu, I will have a look once I am back (next week).

@slvrtrn slvrtrn self-assigned this May 29, 2023
@slvrtrn
Copy link
Collaborator

slvrtrn commented May 29, 2023

@taylorchu, I checked it with 1.1.6, and it seems to be working as expected.

image

Note that there is an option to include the current month in the result set, and it generates two different queries based on that:

Without "include this month" (May will be excluded):

SELECT `default`.`past_test`.`i` AS `i`, `default`.`past_test`.`when` AS `when` 
FROM `default`.`past_test` 
WHERE (`default`.`past_test`.`when` >= toStartOfMonth(toDateTime((CAST(now() AS timestamp) + INTERVAL -1 month))) 
AND `default`.`past_test`.`when` < toStartOfMonth(toDateTime(now()))) 
LIMIT 2000

With "include this month" (May will be included):

SELECT `default`.`past_test`.`i` AS `i`, `default`.`past_test`.`when` AS `when` 
FROM `default`.`past_test` 
WHERE (`default`.`past_test`.`when` >= toStartOfMonth(toDateTime((CAST(now() AS timestamp) + INTERVAL -1 month))) 
AND `default`.`past_test`.`when` < toStartOfMonth(toDateTime((CAST(now() AS timestamp) + INTERVAL 1 month)))) 
LIMIT 2000

^ see the difference in the upper bound

`when` < toStartOfMonth(toDateTime(now()))

-- less than the 1st of May

vs

`when` < toStartOfMonth(toDateTime((CAST(now() AS timestamp) + INTERVAL 1 month)))

-- less than the 1st of June

My dataset was defined like the following:

CREATE TABLE past_test (i UInt64, when DATETIME) 
ENGINE MergeTree 
ORDER BY i;

INSERT INTO past_test 
SELECT number, now() - toIntervalMinute(randCanonical() * 1000000) 
FROM system.numbers LIMIT 100000;

if the issue is still reproducible on your end using 1.1.6, even with "include this month" enabled, can you please share the minimal dataset that can help me investigate this issue?

@taylorchu
Copy link
Author

yes. this is still reproduced in 1.1.6.

My timestamp column is timestamp DateTime64(3).

CREATE DATABASE IF NOT EXISTS app;
CREATE TABLE IF NOT EXISTS app.logs
(
  `timestamp` DateTime64(3),
  `host` LowCardinality(String),
  `namespace_name` LowCardinality(String),
  `pod_name` LowCardinality(String),
  `container_name` LowCardinality(String),
  `stream` LowCardinality(String),
  `labels_app` LowCardinality(String),
  `labels_role` LowCardinality(String),
  `message` String
)
ENGINE = MergeTree
PARTITION BY toStartOfHour(timestamp)
ORDER BY (labels_app, pod_name, container_name, timestamp)
TTL toDateTime(timestamp) + INTERVAL 31 DAY;

@slvrtrn
Copy link
Collaborator

slvrtrn commented May 30, 2023

@taylorchu, after some investigation, it turned out this is due to the issue related to ClickHouse itself: ClickHouse/ClickHouse#50353

There is a workaround available, I will check if it's possible to squeeze it into the :month method (that's where the toStartOfMonth call is coming from), cause it's the only one for all the Date* types.

@slvrtrn
Copy link
Collaborator

slvrtrn commented May 30, 2023

@taylorchu, there is a PR that fixes this odd behavior on the CH server side: ClickHouse/ClickHouse#50280.

@taylorchu
Copy link
Author

Thanks! it seems like a critical bug.

@taylorchu
Copy link
Author

After updating to 23.5, "quarter" and "year" still do not work, but the others do.

@taylorchu
Copy link
Author

Actually that is because I did not include "current quarter and year". This is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants