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

Aggregate queries using group by doesn't return the correct number of rows #203

Open
jeffquach opened this issue Feb 28, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@jeffquach
Copy link

jeffquach commented Feb 28, 2025

Describe the bug

When running a query with group by and built-in functions in Clickhouse the correct result is not returned compared with running the exact same query on the Clickhouse command line. This was tested within actix-web.

Steps to reproduce

  1. Run query on the Clickhouse CLI.
  2. Execute database query within route handler in actix-web

Expected behaviour

Only 1 - 3 rows are returned when the limit number should be returned.

Code example

use actix_web::{get, web, HttpResponse, Responder};
use clickhouse::{Client, Row};
use serde::{Deserialize, Serialize};
use time::{OffsetDateTime};

#[derive(Debug, Row, Deserialize, Serialize)]
struct StockRow<'a> {
    ticker: &'a str,
    #[serde(with = "clickhouse::serde::time::datetime")]
    stock_timestamp: OffsetDateTime,
    open: f32,
    high: f32,
    low: f32,
    close: f32,
    volume: u32,
}

#[get("/api/data")]
async fn api_data(
    client: web::Data<Client>,
    params: web::Query<QueryParams>,
) -> Result<impl Responder, actix_web::Error> {
    let ticker = &params.ticker;
    let limit: i16 = match params.limit {
        Some(limit) => limit,
        None => 100,
    };

    let mut cursor = client
        .query("SELECT ticker, toStartOfMinute(stock_timestamp) stock_timestamp, argMin(open, stock_timestamp) AS open, 
        MAX(high) AS high, 
        MIN(low) AS low, 
        argMax(close, stock_timestamp) AS close, 
        SUM(volume) AS volume FROM stocks WHERE ticker = ? GROUP BY ticker, stock_timestamp ORDER BY stock_timestamp LIMIT ?")
        .bind(ticker)
        .bind(limit)
        .fetch::<StockRow<'_>>()
        .unwrap();

    while let Ok(Some(row)) = cursor.next().await {
        println!("{:?}", row);
    }

    Ok(HttpResponse::Ok().body(""))
}

Error log

No errors, incorrect result is returned.

Query log

This query can be tested using the Clickhouse CLI:

SELECT 
    ticker, 
    toStartOfMinute(stock_timestamp) AS stock_timestamp, 
    argMin(open, stock_timestamp) AS open, 
    MAX(high) AS high, 
    MIN(low) AS low, 
    argMax(close, stock_timestamp) AS close, 
    SUM(volume) AS volume 
FROM stocks 
WHERE ticker = 'NVDA'
GROUP BY ticker, stock_timestamp 
ORDER BY stock_timestamp 
LIMIT 100;

Configuration

Environment

  • Client version: 0.13.1 (Clickhouse client CLI: 25.1.5.31)
  • OS: Ubuntu 24.04.2 LTS

ClickHouse server

  • ClickHouse Server version: 25.1.2
  • ClickHouse Server non-default settings, if any:
  • CREATE TABLE statements for tables involved:
CREATE TABLE IF NOT EXISTS test_data.stocks (
    ticker LowCardinality(String),
    stock_timestamp DateTime CODEC(Delta, ZSTD),
    open Float32 CODEC(ZSTD),
    high Float32 CODEC(ZSTD),
    low Float32 CODEC(ZSTD),
    close Float32 CODEC(ZSTD),
    volume UInt32 CODEC(ZSTD),
    timestamp DateTime CODEC(Delta, ZSTD)
) ENGINE = MergeTree()
PARTITION BY toYYYYMM(stock_timestamp)
ORDER BY (ticker, stock_timestamp);

Sample data was outputted as JSON from Clickhouse:

{
	"row_1": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:00:00","open":124.91,"high":124.93,"low":124.3,"close":124.5,"volume":20040123,"timestamp":"2025-02-27 14:30:34"},
	"row_2": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:00:10","open":124.52,"high":125.06,"low":124.36,"close":124.42,"volume":157265,"timestamp":"2025-02-27 14:30:34"},
	"row_3": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:00:20","open":124.42,"high":124.85,"low":124.3,"close":124.44,"volume":46094,"timestamp":"2025-02-27 14:30:34"},
	"row_4": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:00:30","open":124.53,"high":124.62,"low":124.37,"close":124.41,"volume":19278,"timestamp":"2025-02-27 14:30:34"},
	"row_5": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:00:40","open":124.49,"high":124.56,"low":124.4,"close":124.4,"volume":54513,"timestamp":"2025-02-27 14:30:34"},
	"row_6": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:00:50","open":124.42,"high":124.51,"low":124.38,"close":124.51,"volume":19510,"timestamp":"2025-02-27 14:30:34"},
	"row_7": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:01:00","open":124.52,"high":124.57,"low":124.41,"close":124.43,"volume":33862,"timestamp":"2025-02-27 14:30:34"},
	"row_8": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:01:10","open":124.48,"high":124.5,"low":124.25,"close":124.4,"volume":44072,"timestamp":"2025-02-27 14:30:34"},
	"row_9": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:01:20","open":124.41,"high":124.41,"low":124.28,"close":124.36,"volume":21589,"timestamp":"2025-02-27 14:30:34"},
	"row_10": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:01:30","open":124.35,"high":124.43,"low":124.29,"close":124.29,"volume":46371,"timestamp":"2025-02-27 14:30:34"},
	"row_11": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:01:40","open":124.29,"high":124.47,"low":124.29,"close":124.42,"volume":58016,"timestamp":"2025-02-27 14:30:34"},
	"row_12": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:01:50","open":124.43,"high":124.5,"low":124.42,"close":124.5,"volume":13858,"timestamp":"2025-02-27 14:30:34"},
	"row_13": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:02:00","open":124.49,"high":124.71,"low":124.49,"close":124.71,"volume":34373,"timestamp":"2025-02-27 14:30:34"},
	"row_14": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:02:10","open":124.68,"high":124.75,"low":124.64,"close":124.7,"volume":53706,"timestamp":"2025-02-27 14:30:34"},
	"row_15": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:02:20","open":124.72,"high":124.79,"low":124.7,"close":124.79,"volume":8585,"timestamp":"2025-02-27 14:30:34"},
	"row_16": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:02:30","open":124.75,"high":124.85,"low":124.74,"close":124.85,"volume":27377,"timestamp":"2025-02-27 14:30:34"},
	"row_17": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:02:40","open":124.84,"high":124.91,"low":124.83,"close":124.88,"volume":47918,"timestamp":"2025-02-27 14:30:34"},
	"row_18": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:02:50","open":124.83,"high":124.9,"low":124.77,"close":124.83,"volume":37207,"timestamp":"2025-02-27 14:30:34"},
	"row_19": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:03:00","open":124.84,"high":124.95,"low":124.83,"close":124.94,"volume":57767,"timestamp":"2025-02-27 14:30:34"},
	"row_20": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:03:10","open":124.93,"high":124.97,"low":124.65,"close":124.95,"volume":459171,"timestamp":"2025-02-27 14:30:34"},
	"row_21": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:03:20","open":124.91,"high":124.93,"low":124.91,"close":124.92,"volume":23759,"timestamp":"2025-02-27 14:30:34"},
	"row_22": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:03:30","open":124.91,"high":124.93,"low":124.91,"close":124.91,"volume":19199,"timestamp":"2025-02-27 14:30:34"},
	"row_23": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:03:40","open":124.9,"high":124.93,"low":124.83,"close":124.83,"volume":58486,"timestamp":"2025-02-27 14:30:34"},
	"row_24": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:03:50","open":124.83,"high":124.85,"low":124.8,"close":124.84,"volume":8143,"timestamp":"2025-02-27 14:30:34"},
	"row_25": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:04:00","open":124.83,"high":124.84,"low":124.81,"close":124.83,"volume":19816,"timestamp":"2025-02-27 14:30:34"},
	"row_26": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:04:10","open":124.83,"high":124.84,"low":124.82,"close":124.83,"volume":14417,"timestamp":"2025-02-27 14:30:34"},
	"row_27": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:04:20","open":124.84,"high":124.84,"low":124.7,"close":124.7,"volume":18907,"timestamp":"2025-02-27 14:30:34"},
	"row_28": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:04:30","open":124.7,"high":124.87,"low":124.7,"close":124.85,"volume":34307,"timestamp":"2025-02-27 14:30:34"},
	"row_29": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:04:40","open":124.8,"high":124.92,"low":124.65,"close":124.9,"volume":7008442,"timestamp":"2025-02-27 14:30:34"},
	"row_30": {"ticker":"NVDA","stock_timestamp":"2025-01-30 21:04:50","open":124.86,"high":124.93,"low":124.84,"close":124.89,"volume":9378,"timestamp":"2025-02-27 14:30:34"}                                                                     
}
@jeffquach jeffquach added the bug Something isn't working label Feb 28, 2025
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

1 participant