Skip to content

Commit 31c243b

Browse files
authored
fix: set query_kind earlier to ensure it takes effect. (#17302)
maybe related to the performance regression reported in issue 17284.
1 parent a37a790 commit 31c243b

File tree

2 files changed

+20
-9
lines changed

2 files changed

+20
-9
lines changed

src/query/sql/src/planner/planner.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ impl Planner {
223223
#[fastrace::trace]
224224
pub async fn plan_stmt(&mut self, stmt: &Statement, attach_query: bool) -> Result<Plan> {
225225
let start = Instant::now();
226+
let query_kind = get_query_kind(stmt);
226227
let settings = self.ctx.get_settings();
227228
// Step 3: Bind AST with catalog, and generate a pure logical SExpr
228229
let name_resolution_ctx = NameResolutionContext::try_from(settings.as_ref())?;
@@ -243,8 +244,7 @@ impl Planner {
243244
info!("logical plan from cache, time used: {:?}", start.elapsed());
244245
if attach_query {
245246
// update for clickhouse handler
246-
self.ctx
247-
.attach_query_str(get_query_kind(stmt), stmt.to_mask_sql());
247+
self.ctx.attach_query_str(query_kind, stmt.to_mask_sql());
248248
}
249249
return Ok(plan.plan);
250250
}
@@ -260,11 +260,14 @@ impl Planner {
260260
)
261261
.with_subquery_executor(self.query_executor.clone());
262262

263-
// Indicate binder there is no need to collect column statistics for the binding table.
263+
// must attach before bind, because ParquetRSTable::create used it.
264+
if attach_query {
265+
self.ctx.attach_query_str(query_kind, stmt.to_mask_sql());
266+
}
264267
let plan = binder.bind(stmt).await?;
268+
// attach again to avoid the query kind is overwritten by the subquery
265269
if attach_query {
266-
self.ctx
267-
.attach_query_str(get_query_kind(stmt), stmt.to_mask_sql());
270+
self.ctx.attach_query_str(query_kind, stmt.to_mask_sql());
268271
}
269272

270273
// Step 4: Optimize the SExpr with optimizers, and generate optimized physical SExpr

src/query/storages/parquet/src/parquet_rs/parquet_table/table.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use databend_common_catalog::table::DummyColumnStatisticsProvider;
3434
use databend_common_catalog::table::Table;
3535
use databend_common_catalog::table::TableStatistics;
3636
use databend_common_catalog::table_context::TableContext;
37+
use databend_common_exception::ErrorCode;
3738
use databend_common_exception::Result;
3839
use databend_common_expression::TableField;
3940
use databend_common_expression::TableSchema;
@@ -141,10 +142,17 @@ impl ParquetRSTable {
141142

142143
// If the query is `COPY`, we don't need to collect column statistics.
143144
// It's because the only transform could be contained in `COPY` command is projection.
144-
let need_stats_provider = !matches!(
145-
query_kind,
146-
QueryKind::CopyIntoTable | QueryKind::CopyIntoLocation
147-
);
145+
let need_stats_provider = match query_kind {
146+
QueryKind::CopyIntoTable | QueryKind::CopyIntoLocation => true,
147+
QueryKind::Unknown => {
148+
// add this branch to ensure query_kind is set
149+
return Err(ErrorCode::Internal(
150+
"Unexpected QueryKind::Unknown: query_kind was not properly set before calling ParquetRSTable::create.",
151+
));
152+
}
153+
_ => false,
154+
};
155+
148156
let max_threads = settings.get_max_threads()? as usize;
149157
let max_memory_usage = settings.get_max_memory_usage()?;
150158

0 commit comments

Comments
 (0)