Skip to content

refactor: Remove unused re-calculate table statistic logic when creat… #18913

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −70
Open

refactor: Remove unused re-calculate table statistic logic when creat… #18913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/query/ast/tests/it/testdata/stmt.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15370,9 +15370,9 @@ AlterTable(


---------- Input ----------
ALTER TABLE t SET OPTIONS(SNAPSHOT_LOCATION='1/7/_ss/101fd790dbbe4238a31a8f2e2f856179_v4.mpk',block_per_segment = 500);
ALTER TABLE t SET OPTIONS(block_per_segment = 500);
---------- Output ---------
ALTER TABLE t SET OPTIONS (block_per_segment = '500', snapshot_location = '1/7/_ss/101fd790dbbe4238a31a8f2e2f856179_v4.mpk')
ALTER TABLE t SET OPTIONS (block_per_segment = '500')
---------- AST ------------
AlterTable(
AlterTableStmt {
Expand Down Expand Up @@ -15401,7 +15401,6 @@ AlterTable(
action: SetOptions {
set_options: {
"block_per_segment": "500",
"snapshot_location": "1/7/_ss/101fd790dbbe4238a31a8f2e2f856179_v4.mpk",
},
},
},
Expand Down
2 changes: 0 additions & 2 deletions src/query/ee/src/attach_table/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use databend_enterprise_attach_table::AttachTableHandlerWrapper;
use databend_storages_common_cache::LoadParams;
use databend_storages_common_table_meta::meta::TableSnapshot;
use databend_storages_common_table_meta::meta::Versioned;
use databend_storages_common_table_meta::table::OPT_KEY_SNAPSHOT_LOCATION;

pub struct RealAttachTableHandler {}
#[async_trait::async_trait]
Expand Down Expand Up @@ -66,7 +65,6 @@ impl AttachTableHandler for RealAttachTableHandler {
let root = info.root();
let snapshot_loc = snapshot_full_path[root.len()..].to_string();
let mut options = plan.options.clone();
options.insert(OPT_KEY_SNAPSHOT_LOCATION.to_string(), snapshot_loc.clone());

let params = LoadParams {
location: snapshot_loc.clone(),
Expand Down
4 changes: 0 additions & 4 deletions src/query/ee/src/stream/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use databend_storages_common_table_meta::table::OPT_KEY_CHANGE_TRACKING;
use databend_storages_common_table_meta::table::OPT_KEY_CHANGE_TRACKING_BEGIN_VER;
use databend_storages_common_table_meta::table::OPT_KEY_DATABASE_ID;
use databend_storages_common_table_meta::table::OPT_KEY_MODE;
use databend_storages_common_table_meta::table::OPT_KEY_SNAPSHOT_LOCATION;
use databend_storages_common_table_meta::table::OPT_KEY_SOURCE_DATABASE_ID;
use databend_storages_common_table_meta::table::OPT_KEY_SOURCE_TABLE_ID;
use databend_storages_common_table_meta::table::OPT_KEY_TABLE_VER;
Expand Down Expand Up @@ -128,9 +127,6 @@ impl StreamHandler for RealStreamHandler {
options.insert(OPT_KEY_SOURCE_DATABASE_ID.to_owned(), db_id.to_string());
options.insert(OPT_KEY_SOURCE_TABLE_ID.to_string(), table_id.to_string());
options.insert(OPT_KEY_TABLE_VER.to_string(), change_desc.seq.to_string());
if let Some(snapshot_loc) = change_desc.location {
options.insert(OPT_KEY_SNAPSHOT_LOCATION.to_string(), snapshot_loc);
}

let req = CreateTableReq {
create_option: plan.create_option,
Expand Down
7 changes: 0 additions & 7 deletions src/query/service/src/interpreters/common/stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use databend_common_storages_fuse::TableContext;
use databend_common_storages_stream::stream_table::StreamTable;
use databend_storages_common_table_meta::table::OPT_KEY_DATABASE_ID;
use databend_storages_common_table_meta::table::OPT_KEY_DATABASE_NAME;
use databend_storages_common_table_meta::table::OPT_KEY_SNAPSHOT_LOCATION;
use databend_storages_common_table_meta::table::OPT_KEY_SOURCE_DATABASE_ID;
use databend_storages_common_table_meta::table::OPT_KEY_TABLE_NAME;
use databend_storages_common_table_meta::table::OPT_KEY_TABLE_VER;
Expand Down Expand Up @@ -58,9 +57,6 @@ pub async fn dml_build_update_stream_req(
let table_version = inner_fuse.get_table_info().ident.seq;
let mut options = stream.options().clone();
options.insert(OPT_KEY_TABLE_VER.to_string(), table_version.to_string());
if let Some(snapshot_loc) = inner_fuse.snapshot_loc() {
options.insert(OPT_KEY_SNAPSHOT_LOCATION.to_string(), snapshot_loc);
}

// To be compatible with older versions, set source database id.
if !options.contains_key(OPT_KEY_SOURCE_DATABASE_ID) {
Expand Down Expand Up @@ -118,9 +114,6 @@ pub async fn query_build_update_stream_req(
let table_version = inner_fuse.get_table_info().ident.seq;
let mut options = stream.options().clone();
options.insert(OPT_KEY_TABLE_VER.to_string(), table_version.to_string());
if let Some(snapshot_loc) = inner_fuse.snapshot_loc() {
options.insert(OPT_KEY_SNAPSHOT_LOCATION.to_string(), snapshot_loc);
}
let mut new_table_meta = stream_info.meta.clone();
new_table_meta.options = options;
new_table_meta.updated_on = Utc::now();
Expand Down
7 changes: 0 additions & 7 deletions src/query/service/src/interpreters/interpreter_table_add_column.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use databend_common_storages_stream::stream_table::STREAM_ENGINE;
use databend_storages_common_table_meta::meta::TableSnapshot;
use databend_storages_common_table_meta::meta::Versioned;
use databend_storages_common_table_meta::readers::snapshot_reader::TableSnapshotAccessor;
use databend_storages_common_table_meta::table::OPT_KEY_SNAPSHOT_LOCATION;
use log::info;

use crate::interpreters::interpreter_table_create::is_valid_column;
Expand Down Expand Up @@ -219,12 +218,6 @@ pub(crate) async fn commit_table_meta(
let new_snapshot_location =
generate_new_snapshot(ctx, fuse_tbl, &new_table_meta.schema).await?;

if let Some(new_snapshot_location) = &new_snapshot_location {
new_table_meta.options.insert(
OPT_KEY_SNAPSHOT_LOCATION.to_owned(),
new_snapshot_location.clone(),
);
};

let req = UpdateTableMetaReq {
table_id,
Expand Down
38 changes: 1 addition & 37 deletions src/query/service/src/interpreters/interpreter_table_create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use std::sync::Arc;
use chrono::Utc;
use databend_common_ast::ast::Engine;
use databend_common_base::runtime::GlobalIORuntime;
use databend_common_config::GlobalConfig;
use databend_common_exception::ErrorCode;
use databend_common_exception::Result;
use databend_common_expression::is_internal_column;
Expand Down Expand Up @@ -48,21 +47,16 @@ use databend_common_pipeline_core::always_callback;
use databend_common_pipeline_core::ExecutionInfo;
use databend_common_sql::plans::CreateTablePlan;
use databend_common_sql::DefaultExprBinder;
use databend_common_storages_fuse::io::MetaReaders;
use databend_common_storages_fuse::FuseSegmentFormat;
use databend_common_storages_fuse::FuseStorageFormat;
use databend_common_storages_fuse::FUSE_OPT_KEY_ENABLE_AUTO_ANALYZE;
use databend_common_storages_fuse::FUSE_OPT_KEY_ENABLE_AUTO_VACUUM;
use databend_common_users::RoleCacheManager;
use databend_common_users::UserApiProvider;
use databend_enterprise_attach_table::get_attach_table_handler;
use databend_storages_common_cache::LoadParams;
use databend_storages_common_table_meta::meta::TableSnapshot;
use databend_storages_common_table_meta::meta::Versioned;
use databend_storages_common_table_meta::table::OPT_KEY_COMMENT;
use databend_storages_common_table_meta::table::OPT_KEY_ENABLE_COPY_DEDUP_FULL_PATH;
use databend_storages_common_table_meta::table::OPT_KEY_SEGMENT_FORMAT;
use databend_storages_common_table_meta::table::OPT_KEY_SNAPSHOT_LOCATION;
use databend_storages_common_table_meta::table::OPT_KEY_STORAGE_FORMAT;
use databend_storages_common_table_meta::table::OPT_KEY_STORAGE_PREFIX;
use databend_storages_common_table_meta::table::OPT_KEY_TEMP_PREFIX;
Expand Down Expand Up @@ -348,37 +342,7 @@ impl CreateTableInterpreter {
#[async_backtrace::framed]
async fn create_table(&self) -> Result<PipelineBuildResult> {
let catalog = self.ctx.get_catalog(self.plan.catalog.as_str()).await?;
let mut stat = None;
if !GlobalConfig::instance().query.management_mode {
if let Some(snapshot_loc) = self.plan.options.get(OPT_KEY_SNAPSHOT_LOCATION) {
// using application level data operator is a temp workaround
// please see discussions https://github.com/datafuselabs/databend/pull/10424
let operator = self.ctx.get_application_level_data_operator()?.operator();
let reader = MetaReaders::table_snapshot_reader(operator);

let params = LoadParams {
location: snapshot_loc.clone(),
len_hint: None,
ver: TableSnapshot::VERSION,
put_cache: true,
};

let snapshot = reader.read(&params).await?;
stat = Some(TableStatistics {
number_of_rows: snapshot.summary.row_count,
data_bytes: snapshot.summary.uncompressed_byte_size,
compressed_data_bytes: snapshot.summary.compressed_byte_size,
index_data_bytes: snapshot.summary.index_size,
bloom_index_size: snapshot.summary.bloom_index_size,
ngram_index_size: snapshot.summary.ngram_index_size,
inverted_index_size: snapshot.summary.inverted_index_size,
vector_index_size: snapshot.summary.vector_index_size,
virtual_column_size: snapshot.summary.virtual_column_size,
number_of_segments: Some(snapshot.segments.len() as u64),
number_of_blocks: Some(snapshot.summary.block_count),
});
}
}
let stat = None;
let req = if let Some(storage_prefix) = self.plan.options.get(OPT_KEY_STORAGE_PREFIX) {
self.build_attach_request(storage_prefix).await
} else {
Expand Down
10 changes: 0 additions & 10 deletions src/query/service/src/interpreters/interpreter_table_set_options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ use databend_storages_common_table_meta::table::OPT_KEY_CHANGE_TRACKING_BEGIN_VE
use databend_storages_common_table_meta::table::OPT_KEY_CLUSTER_TYPE;
use databend_storages_common_table_meta::table::OPT_KEY_DATABASE_ID;
use databend_storages_common_table_meta::table::OPT_KEY_SEGMENT_FORMAT;
use databend_storages_common_table_meta::table::OPT_KEY_SNAPSHOT_LOCATION;
use databend_storages_common_table_meta::table::OPT_KEY_STORAGE_FORMAT;
use databend_storages_common_table_meta::table::OPT_KEY_TEMP_PREFIX;
use log::error;
Expand Down Expand Up @@ -172,15 +171,6 @@ impl Interpreter for SetOptionsInterpreter {
is_valid_bloom_index_columns(&self.plan.set_options, table.schema())?;
is_valid_approx_distinct_columns(&self.plan.set_options, table.schema())?;

if let Some(new_snapshot_location) =
set_segment_format(self.ctx.clone(), table.clone(), &self.plan.set_options).await?
{
options_map.insert(
OPT_KEY_SNAPSHOT_LOCATION.to_string(),
Some(new_snapshot_location),
);
}

let req = UpsertTableOptionReq {
table_id: table.get_id(),
seq: MatchSeq::Exact(table_version),
Expand Down
Loading