fix: conversion from TableMeta to TableMetaBuilder (#5693)

* refactor: use proc macro to generate conversion between TableMeta and TableMetaBuilder

* chore: format

* fix/partition-key-index:
 ### Update `TableMeta` and Add Partition and Alter Table Tests

 - **`metadata.rs`**: Modified `new_meta_builder` method in `TableMeta` to manually remove `value_indices` by setting it to `None` in the `TableMetaBuilder`.
 - **`partition_and_alter.result` & `partition_and_alter.sql`**: Added new test cases for creating, inserting, selecting, altering, and dropping a partitioned table `molestiAe`. These tests verify partitioning on the `sImiLiQUE` column and altering the table with a TTL
 setting.

fix/partition-key-index:
 ### Remove Obsolete TODO Comment in `metadata.rs`

 - Removed an outdated TODO comment regarding the `new_meta_builder` function in `src/table/src/metadata.rs`.

chore: check struct name in derive_meta_builder

refactor: Simplify TableMeta struct name check in macro

refactor: Improve ToMetaBuilder derive macro validation and error handling

refactor: Enforce ToMetaBuilder macro for table::metadata::TableMeta struct

* fix/partition-key-index:
 Update `partition_and_alter.sql` to modify TTL setting

 - Modified the TTL setting for the `molestiAe` table to '1d' in `partition_and_alter.sql`.

* fix: sqlness

* fix/partition-key-index:
 ### Update `TableMeta` and Test File Structure

 - **Enhancement**: Added a note in `metadata.rs` to always use `new_meta_builder` for creating `TableMetaBuilder`.
 - **Refactor**: Renamed test result and SQL files for better organization:
   - `partition_and_alter.result` to `alter/partition_and_alter.result`
   - `partition_and_alter.sql` to `alter/partition_and_alter.sql`

* refactor: Simplify `derive_meta_builder` by initializing fields with `Default::default()`

* fix/partition-key-index:
 ### Commit Summary

 - **Refactor `TableMetaBuilder` Initialization**:
   - Replaced `TableMetaBuilder::default()` with `TableMetaBuilder::empty()` across multiple files for initializing `TableMetaBuilder` instances.
   - Affected files include:
     - `src/catalog/src/system_schema.rs`
     - `src/common/meta/src/key/test_utils.rs`
     - `src/operator/src/req_convert/insert/fill_impure_default.rs`
     - `src/query/src/log_query/planner.rs`
     - `src/query/src/promql/planner.rs`
     - `src/query/src/range_select/plan_rewrite.rs`
     - `src/query/src/sql/show_create_table.rs`
     - `src/table/src/test_util/memtable.rs`
     - `src/table/src/test_util/table_info.rs`

 - **Enhance `TableMetaBuilder`**:
   - Added `custom_constructor` to `TableMeta` and implemented an `empty` method for `TableMetaBuilder`.
   - Modified `TableMetaBuilder` to include a `new_external_table` method with default values.
   - Updated `src/table/src/metadata.rs` to reflect these changes.

 - **Add Testing Feature**:
   - Introduced a conditional compilation for `test_util` in `src/table/src/lib.rs` to include testing utilities when the `testing` feature is enabled.

 - **Update `Cargo.toml`**:
   - Enabled the `testing` feature for the `table` module in `src/common/meta/Cargo.toml`.

 - **Modify `NumbersTable` Initialization**:
   - Replaced `TableMetaBuilder` with direct `TableMeta` struct initialization in `src/table/src/table/numbers.rs`.

 - **Test Result Update**:
   - Updated test results in `tests/cases/standalone/common/alter/partition_and_alter.result` to reflect changes in table meta handling.

* fix: rename default to empty

* docs: add doc for TableMetaBuilder::empty

* chore: Update src/table/src/metadata.rs

---------

Co-authored-by: Yingwen <realevenyag@gmail.com>
This commit is contained in:
Lei, HUANG
2025-03-13 14:30:16 +08:00
committed by GitHub
parent e0ff701e51
commit e375a18011
18 changed files with 192 additions and 57 deletions

View File

@@ -77,7 +77,7 @@ trait SystemSchemaProviderInner {
fn system_table(&self, name: &str) -> Option<SystemTableRef>;
fn table_info(catalog_name: String, table: &SystemTableRef) -> TableInfoRef {
let table_meta = TableMetaBuilder::default()
let table_meta = TableMetaBuilder::empty()
.schema(table.schema())
.primary_key_indices(vec![])
.next_column_id(0)

View File

@@ -18,11 +18,13 @@ mod print_caller;
mod range_fn;
mod stack_trace_debug;
mod utils;
use aggr_func::{impl_aggr_func_type_store, impl_as_aggr_func_creator};
use print_caller::process_print_caller;
use proc_macro::TokenStream;
use quote::quote;
use range_fn::process_range_fn;
use syn::{parse_macro_input, DeriveInput};
use syn::{parse_macro_input, Data, DeriveInput, Fields};
use crate::admin_fn::process_admin_fn;
@@ -136,3 +138,51 @@ pub fn print_caller(args: TokenStream, input: TokenStream) -> TokenStream {
pub fn stack_trace_debug(args: TokenStream, input: TokenStream) -> TokenStream {
stack_trace_debug::stack_trace_style_impl(args.into(), input.into()).into()
}
/// Generates implementation for `From<&TableMeta> for TableMetaBuilder`
#[proc_macro_derive(ToMetaBuilder)]
pub fn derive_meta_builder(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let Data::Struct(data_struct) = input.data else {
panic!("ToMetaBuilder can only be derived for structs");
};
let Fields::Named(fields) = data_struct.fields else {
panic!("ToMetaBuilder can only be derived for structs with named fields");
};
// Check that this is being applied to TableMeta struct
if input.ident != "TableMeta" {
panic!("ToMetaBuilder can only be derived for TableMeta struct");
}
let field_init = fields.named.iter().map(|field| {
let field_name = field.ident.as_ref().unwrap();
quote! {
#field_name: Default::default(),
}
});
let field_assignments = fields.named.iter().map(|field| {
let field_name = field.ident.as_ref().unwrap();
quote! {
builder.#field_name(meta.#field_name.clone());
}
});
let gen = quote! {
impl From<&TableMeta> for TableMetaBuilder {
fn from(meta: &TableMeta) -> Self {
let mut builder = Self {
#(#field_init)*
};
#(#field_assignments)*
builder
}
}
};
gen.into()
}

View File

@@ -61,7 +61,7 @@ snafu.workspace = true
sqlx = { workspace = true, optional = true }
store-api.workspace = true
strum.workspace = true
table.workspace = true
table = { workspace = true, features = ["testing"] }
tokio.workspace = true
tokio-postgres = { workspace = true, optional = true }
tonic.workspace = true

View File

@@ -40,7 +40,7 @@ pub fn new_test_table_info_with_name<I: IntoIterator<Item = u32>>(
.build()
.unwrap();
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(Arc::new(schema))
.primary_key_indices(vec![0])
.engine("engine")

View File

@@ -41,7 +41,7 @@ pub fn new_test_table_info_with_name<I: IntoIterator<Item = u32>>(
.build()
.unwrap();
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(Arc::new(schema))
.primary_key_indices(vec![0])
.engine("engine")

View File

@@ -183,7 +183,7 @@ mod tests {
pub fn new_table_info() -> TableInfo {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")

View File

@@ -60,7 +60,7 @@ pub fn new_test_table_info(
.build()
.unwrap();
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(Arc::new(schema))
.primary_key_indices(vec![0])
.engine("engine")

View File

@@ -433,7 +433,7 @@ mod tests {
let catalog_list = MemoryCatalogManager::with_default_setup();
for (schema_name, table_name) in table_name_tuples {
let schema = mock_schema();
let table_meta = TableMetaBuilder::default()
let table_meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![2])
.value_indices(vec![0])

View File

@@ -2992,7 +2992,7 @@ mod test {
));
}
let schema = Arc::new(Schema::new(columns));
let table_meta = TableMetaBuilder::default()
let table_meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices((0..num_tag).collect())
.value_indices((num_tag + 1..num_tag + 1 + num_field).collect())
@@ -3055,7 +3055,7 @@ mod test {
true,
));
let schema = Arc::new(Schema::new(columns));
let table_meta = TableMetaBuilder::default()
let table_meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices((0..num_tag).collect())
.next_column_id(1024)
@@ -4031,7 +4031,7 @@ mod test {
),
];
let schema = Arc::new(Schema::new(columns));
let table_meta = TableMetaBuilder::default()
let table_meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.value_indices(vec![2])

View File

@@ -610,7 +610,7 @@ mod test {
));
}
let schema = Arc::new(Schema::new(columns));
let table_meta = TableMetaBuilder::default()
let table_meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices((0..5).collect())
.value_indices((6..11).collect())

View File

@@ -291,7 +291,7 @@ mod tests {
.extra_options
.insert("compaction.type".to_string(), "twcs".to_string());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(table_schema)
.primary_key_indices(vec![0, 1])
.value_indices(vec![2, 3])
@@ -362,7 +362,7 @@ WITH(
let _ = options
.extra_options
.insert(FILE_TABLE_FORMAT_KEY.to_string(), "csv".to_string());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(table_schema)
.primary_key_indices(vec![])
.engine("file".to_string())

View File

@@ -24,6 +24,7 @@ pub mod stats;
pub mod table;
pub mod table_name;
pub mod table_reference;
#[cfg(any(test, feature = "testing"))]
pub mod test_util;
pub use crate::error::{Error, Result};

View File

@@ -17,6 +17,7 @@ use std::sync::Arc;
use chrono::{DateTime, Utc};
use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME};
use common_macro::ToMetaBuilder;
use common_query::AddColumnLocation;
use datafusion_expr::TableProviderFilterPushDown;
pub use datatypes::error::{Error as ConvertError, Result as ConvertResult};
@@ -112,9 +113,8 @@ pub struct TableIdent {
/// The table metadata.
///
/// Note: if you add new fields to this struct, please ensure 'new_meta_builder' function works.
/// TODO(dennis): find a better way to ensure 'new_meta_builder' works when adding new fields.
#[derive(Clone, Debug, Builder, PartialEq, Eq)]
#[builder(pattern = "mutable")]
#[derive(Clone, Debug, Builder, PartialEq, Eq, ToMetaBuilder)]
#[builder(pattern = "mutable", custom_constructor)]
pub struct TableMeta {
pub schema: SchemaRef,
/// The indices of columns in primary key. Note that the index of timestamp column
@@ -136,6 +136,24 @@ pub struct TableMeta {
pub partition_key_indices: Vec<usize>,
}
impl TableMetaBuilder {
/// Note: Please always use [new_meta_builder] to create new [TableMetaBuilder].
#[cfg(any(test, feature = "testing"))]
pub fn empty() -> Self {
Self {
schema: None,
primary_key_indices: None,
value_indices: None,
engine: None,
region_numbers: None,
next_column_id: None,
options: None,
created_on: None,
partition_key_indices: None,
}
}
}
impl TableMetaBuilder {
fn default_value_indices(&self) -> std::result::Result<Vec<usize>, String> {
match (&self.primary_key_indices, &self.schema) {
@@ -151,11 +169,15 @@ impl TableMetaBuilder {
pub fn new_external_table() -> Self {
Self {
schema: None,
primary_key_indices: Some(Vec::new()),
value_indices: Some(Vec::new()),
engine: None,
region_numbers: Some(Vec::new()),
next_column_id: Some(0),
..Default::default()
options: None,
created_on: None,
partition_key_indices: None,
}
}
}
@@ -492,20 +514,11 @@ impl TableMeta {
Ok(desc)
}
/// Create a [`TableMetaBuilder`].
///
/// Note: please always use this function to create the builder.
/// Create a [`TableMetaBuilder`] from the current TableMeta.
fn new_meta_builder(&self) -> TableMetaBuilder {
let mut builder = TableMetaBuilder::default();
let _ = builder
.schema(self.schema.clone())
.primary_key_indices(self.primary_key_indices.clone())
.engine(&self.engine)
.options(self.options.clone())
.created_on(self.created_on)
.region_numbers(self.region_numbers.clone())
.next_column_id(self.next_column_id);
let mut builder = TableMetaBuilder::from(self);
// Manually remove value_indices.
builder.value_indices = None;
builder
}
@@ -1246,7 +1259,7 @@ mod tests {
#[test]
fn test_raw_convert() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1328,7 +1341,7 @@ mod tests {
#[test]
fn test_add_columns() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1353,7 +1366,7 @@ mod tests {
#[test]
fn test_add_columns_multiple_times() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1395,7 +1408,7 @@ mod tests {
#[test]
fn test_remove_columns() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema.clone())
.primary_key_indices(vec![0])
.engine("engine")
@@ -1451,7 +1464,7 @@ mod tests {
.build()
.unwrap(),
);
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema.clone())
.primary_key_indices(vec![1])
.engine("engine")
@@ -1487,7 +1500,7 @@ mod tests {
#[test]
fn test_add_existing_column() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1534,7 +1547,7 @@ mod tests {
#[test]
fn test_add_different_type_column() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1561,7 +1574,7 @@ mod tests {
#[test]
fn test_add_invalid_column() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1593,7 +1606,7 @@ mod tests {
#[test]
fn test_remove_unknown_column() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1615,7 +1628,7 @@ mod tests {
#[test]
fn test_change_unknown_column_data_type() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1640,7 +1653,7 @@ mod tests {
#[test]
fn test_remove_key_column() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1674,7 +1687,7 @@ mod tests {
#[test]
fn test_change_key_column_data_type() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1714,7 +1727,7 @@ mod tests {
#[test]
fn test_alloc_new_column() {
let schema = Arc::new(new_test_schema());
let mut meta = TableMetaBuilder::default()
let mut meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1733,7 +1746,7 @@ mod tests {
#[test]
fn test_add_columns_with_location() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")
@@ -1761,7 +1774,7 @@ mod tests {
#[test]
fn test_modify_column_fulltext_options() {
let schema = Arc::new(new_test_schema());
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![0])
.engine("engine")

View File

@@ -30,7 +30,7 @@ use store_api::data_source::DataSource;
use store_api::storage::ScanRequest;
use crate::metadata::{
FilterPushDownType, TableId, TableInfoBuilder, TableInfoRef, TableMetaBuilder, TableType,
FilterPushDownType, TableId, TableInfoBuilder, TableInfoRef, TableMeta, TableType,
};
use crate::{Table, TableRef};
@@ -71,14 +71,18 @@ impl NumbersTable {
}
pub fn table_info(table_id: TableId, name: String, engine: String) -> TableInfoRef {
let table_meta = TableMetaBuilder::default()
.schema(Self::schema())
.region_numbers(vec![0])
.primary_key_indices(vec![0])
.next_column_id(1)
.engine(engine)
.build()
.unwrap();
let table_meta = TableMeta {
schema: Self::schema(),
primary_key_indices: vec![0],
value_indices: vec![],
engine,
region_numbers: vec![0],
next_column_id: 1,
options: Default::default(),
created_on: Default::default(),
partition_key_indices: vec![],
};
let table_info = TableInfoBuilder::default()
.table_id(table_id)
.name(name)

View File

@@ -67,7 +67,7 @@ impl MemTable {
) -> TableRef {
let schema = recordbatch.schema.clone();
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![])
.value_indices(vec![])

View File

@@ -23,7 +23,7 @@ pub fn test_table_info(
catalog_name: &str,
schema: SchemaRef,
) -> TableInfo {
let meta = TableMetaBuilder::default()
let meta = TableMetaBuilder::empty()
.schema(schema)
.primary_key_indices(vec![])
.value_indices(vec![])

View File

@@ -0,0 +1,44 @@
CREATE TABLE `molestiAe` (
`sImiLiQUE` FLOAT NOT NULL,
`amEt` TIMESTAMP(6) TIME INDEX,
`EXpLICaBo` DOUBLE,
PRIMARY KEY (`sImiLiQUE`)
) PARTITION ON COLUMNS (`sImiLiQUE`) (
`sImiLiQUE` < -1,
`sImiLiQUE` >= -1 AND `sImiLiQUE` < -0,
`sImiLiQUE` >= 0
);
Affected Rows: 0
INSERT INTO `molestiAe` VALUES
(-2, 0, 0),
(-0.9, 0, 0),
(1, 0, 0);
Affected Rows: 3
SELECT COUNT(*) from `molestiAe`;
+----------+
| count(*) |
+----------+
| 3 |
+----------+
ALTER TABLE `molestiAe` SET 'ttl' = '1d';
Affected Rows: 0
SELECT COUNT(*) from `molestiAe`;
+----------+
| count(*) |
+----------+
| 3 |
+----------+
DROP TABLE `molestiAe`;
Affected Rows: 0

View File

@@ -0,0 +1,23 @@
CREATE TABLE `molestiAe` (
`sImiLiQUE` FLOAT NOT NULL,
`amEt` TIMESTAMP(6) TIME INDEX,
`EXpLICaBo` DOUBLE,
PRIMARY KEY (`sImiLiQUE`)
) PARTITION ON COLUMNS (`sImiLiQUE`) (
`sImiLiQUE` < -1,
`sImiLiQUE` >= -1 AND `sImiLiQUE` < -0,
`sImiLiQUE` >= 0
);
INSERT INTO `molestiAe` VALUES
(-2, 0, 0),
(-0.9, 0, 0),
(1, 0, 0);
SELECT COUNT(*) from `molestiAe`;
ALTER TABLE `molestiAe` SET 'ttl' = '1d';
SELECT COUNT(*) from `molestiAe`;
DROP TABLE `molestiAe`;