Skip to content

Conversation

@youngsofun
Copy link
Member

@youngsofun youngsofun commented Oct 21, 2025

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

The query_state_handler (GET /v1/query/<query_id>) used to share QueryResponse with the page_handler, the difference is that it do not return any data and return at once ( without long polling). This PR modifies query_state_handler to return a standalone struct StateResponse by removing unused fields, aiming to clarify the API.

Motivation

  1. Our understanding of API usage has become clearer: query_state_handler is not intended for drivers, but currently only used within worksheets. Its purpose should be restricted to progress and state fetching by threads other than the driver.
  2. Fields in QueryResponse have evolved gradually over time, leading to the accumulation of unreasonable/ unnecessary fields since its inception.
  3. Many of these fields lack meaningful values for query_state_handler.

Compatibility Considerations

  1. The removed fields (as listed) are unused to my knowledge (cc @hantmac cc @Chasen-Zhang )
  2. for potential unknown usage:
    1. they are not logically relevant for non-driver threads thus should not be used.
    2. For Java/Python/Go clients, missing fields will map to default values when map to structs. As long as these fields were not actually in use, this change should not cause errors.
    3. Most of these fields are inherently optional, so even if clients were (by any chance) using them, they should be able to handle null/empty values. some are already fixed to None before this pr
    // about result/side affect,  should only be known by the driver thread.
    pub data: Arc<BlocksSerializer>,  
    pub affect: Option<QueryAffect>,
    pub session: Option<HttpSessionConf>,
    pub schema: Vec<QueryResponseField>,

   // added  recently
    pub session_id: Option<String>,
    pub result_timeout_secs: Option<u64>,
    pub has_result_set: Option<bool>,

    // already known by caller, from the initial response of req that start the query
    pub id: String,
    pub node_id: String,
    pub stats_uri: Option<String>, 
    pub final_uri: Option<String>, // should only be used by the driver thread.
    pub next_uri: Option<String>,  // not reasonable
    pub kill_uri: Option<String>,

remaining fields

pub struct StateResponse {
    pub state: ExecuteStateKind,
    pub error: Option<QueryError>,
    pub warnings: Vec<String>,
    pub stats: QueryStats,
}

improvement by the way:

  1. partially fixed: These logs are is spamming the output when using worksheet #18874
  2. add better test for http query lifecycle

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Oct 21, 2025
@youngsofun youngsofun marked this pull request as draft October 21, 2025 14:04
@youngsofun youngsofun changed the title refactor: remove useless optional fields in response of query_state_handler. feat: remove useless optional fields in response of query_state_handler. Oct 21, 2025
@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label Oct 21, 2025
@youngsofun youngsofun marked this pull request as ready for review October 21, 2025 16:02
@youngsofun youngsofun merged commit c3ed04d into databendlabs:main Oct 22, 2025
261 of 265 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature this PR introduces a new feature to the codebase pr-refactor this PR changes the code base without new features or bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

These logs are is spamming the output when using worksheet

2 participants