Skip to content

Commit

Permalink
fix(table): return correct table types (GreptimeTeam#2131)
Browse files Browse the repository at this point in the history
* fix(table): return correct table types

Signed-off-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>

* fix: NumbersTable to be Temporary table

Signed-off-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>

* fix(test): fix affected cases

Signed-off-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>

* fix(test): fix affected cases

Signed-off-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>

* fix: fmt

Signed-off-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>

* fix(tests): fix instance_test expected result

* retrigger action

Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>

---------

Signed-off-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>
Signed-off-by: Zhenchi <zhongzc_arch@outlook.com>
Co-authored-by: zhongzc <zhongzc@zhongzcs-MacBook-Pro.local>
  • Loading branch information
zhongzc and zhongzc authored Aug 9, 2023
1 parent b156225 commit 7abe71f
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 47 deletions.
4 changes: 2 additions & 2 deletions src/catalog/src/information_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ impl Table for InformationTable {
unreachable!("Should not call table_info() of InformationTable directly")
}

fn table_type(&self) -> table::metadata::TableType {
TableType::View
fn table_type(&self) -> TableType {
TableType::Temporary
}

async fn scan_to_stream(&self, request: ScanRequest) -> TableResult<SendableRecordBatchStream> {
Expand Down
6 changes: 5 additions & 1 deletion src/catalog/src/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use serde::{Deserialize, Serialize};
use snafu::{ensure, OptionExt, ResultExt};
use store_api::storage::ScanRequest;
use table::engine::{EngineContext, TableEngineRef};
use table::metadata::{TableId, TableInfoRef};
use table::metadata::{TableId, TableInfoRef, TableType};
use table::requests::{
CreateTableRequest, DeleteRequest, InsertRequest, OpenTableRequest, TableOptions,
};
Expand Down Expand Up @@ -71,6 +71,10 @@ impl Table for SystemCatalogTable {
self.0.table_info()
}

fn table_type(&self) -> TableType {
self.0.table_type()
}

async fn delete(&self, request: DeleteRequest) -> TableResult<usize> {
self.0.delete(request).await
}
Expand Down
6 changes: 5 additions & 1 deletion src/frontend/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use partition::splitter::WriteSplitter;
use snafu::prelude::*;
use store_api::storage::{RegionNumber, ScanRequest};
use table::error::TableOperationSnafu;
use table::metadata::{FilterPushDownType, TableInfoRef};
use table::metadata::{FilterPushDownType, TableInfoRef, TableType};
use table::requests::{DeleteRequest, InsertRequest};
use table::Table;
use tokio::sync::RwLock;
Expand Down Expand Up @@ -79,6 +79,10 @@ impl Table for DistTable {
self.table_info.clone()
}

fn table_type(&self) -> TableType {
self.table_info.table_type
}

async fn insert(&self, request: InsertRequest) -> table::Result<usize> {
let inserter = DistInserter::new(
request.catalog_name.clone(),
Expand Down
42 changes: 21 additions & 21 deletions src/query/src/dist_plan/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,15 @@ mod test {

let config = ConfigOptions::default();
let result = DistPlannerAnalyzer {}.analyze(plan, &config).unwrap();
let expected = String::from(
"Distinct:\
\n MergeScan [is_placeholder=false]\
\n Distinct:\
\n Projection: t.number\
\n Filter: t.number < Int32(10)\
\n TableScan: t",
);
let expected = [
"Distinct:",
" MergeScan [is_placeholder=false]",
" Distinct:",
" Projection: t.number",
" Filter: t.number < Int32(10)",
" TableScan: t",
]
.join("\n");
assert_eq!(expected, format!("{:?}", result));
}

Expand All @@ -322,10 +323,11 @@ mod test {

let config = ConfigOptions::default();
let result = DistPlannerAnalyzer {}.analyze(plan, &config).unwrap();
let expected = String::from(
"Aggregate: groupBy=[[]], aggr=[[AVG(t.number)]]\
\n MergeScan [is_placeholder=false]",
);
let expected = [
"Aggregate: groupBy=[[]], aggr=[[AVG(t.number)]]",
" TableScan: t",
]
.join("\n");
assert_eq!(expected, format!("{:?}", result));
}

Expand All @@ -347,11 +349,12 @@ mod test {

let config = ConfigOptions::default();
let result = DistPlannerAnalyzer {}.analyze(plan, &config).unwrap();
let expected = String::from(
"Sort: t.number ASC NULLS LAST\
\n Distinct:\
\n MergeScan [is_placeholder=false]",
);
let expected = [
"Sort: t.number ASC NULLS LAST",
" Distinct:",
" TableScan: t",
]
.join("\n");
assert_eq!(expected, format!("{:?}", result));
}

Expand All @@ -371,10 +374,7 @@ mod test {

let config = ConfigOptions::default();
let result = DistPlannerAnalyzer {}.analyze(plan, &config).unwrap();
let expected = String::from(
"Limit: skip=0, fetch=1\
\n MergeScan [is_placeholder=false]",
);
let expected = ["Limit: skip=0, fetch=1", " TableScan: t"].join("\n");
assert_eq!(expected, format!("{:?}", result));
}
}
6 changes: 5 additions & 1 deletion src/query/src/tests/time_range_filter_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use datatypes::data_type::ConcreteDataType;
use datatypes::schema::{ColumnSchema, Schema, SchemaRef};
use datatypes::vectors::{Int64Vector, TimestampMillisecondVector};
use store_api::storage::ScanRequest;
use table::metadata::{FilterPushDownType, TableInfoRef};
use table::metadata::{FilterPushDownType, TableInfoRef, TableType};
use table::predicate::TimeRangePredicateBuilder;
use table::test_util::MemTable;
use table::Table;
Expand Down Expand Up @@ -61,6 +61,10 @@ impl Table for MemTableWrapper {
self.inner.table_info()
}

fn table_type(&self) -> TableType {
self.inner.table_type()
}

async fn scan_to_stream(
&self,
request: ScanRequest,
Expand Down
4 changes: 1 addition & 3 deletions src/table/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ pub trait Table: Send + Sync {
fn table_info(&self) -> TableInfoRef;

/// Get the type of this table for metadata/catalog purposes.
fn table_type(&self) -> TableType {
TableType::Base
}
fn table_type(&self) -> TableType;

/// Insert values into table.
///
Expand Down
6 changes: 5 additions & 1 deletion src/table/src/table/numbers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl Table for NumbersTable {
.catalog_name(DEFAULT_CATALOG_NAME)
.schema_name(DEFAULT_SCHEMA_NAME)
.table_version(0)
.table_type(TableType::Base)
.table_type(TableType::Temporary)
.meta(
TableMetaBuilder::default()
.schema(self.schema.clone())
Expand All @@ -109,6 +109,10 @@ impl Table for NumbersTable {
)
}

fn table_type(&self) -> TableType {
TableType::Temporary
}

async fn scan_to_stream(&self, request: ScanRequest) -> Result<SendableRecordBatchStream> {
Ok(Box::pin(NumbersStream {
limit: request.limit.unwrap_or(100) as u32,
Expand Down
4 changes: 4 additions & 0 deletions src/table/src/test_util/empty_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl Table for EmptyTable {
self.info.clone()
}

fn table_type(&self) -> TableType {
self.info.table_type
}

async fn insert(&self, _request: InsertRequest) -> Result<usize> {
Ok(0)
}
Expand Down
4 changes: 4 additions & 0 deletions src/table/src/test_util/memtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ impl Table for MemTable {
self.info.clone()
}

fn table_type(&self) -> TableType {
self.info.table_type
}

async fn scan_to_stream(&self, request: ScanRequest) -> Result<SendableRecordBatchStream> {
let df_recordbatch = if let Some(indices) = request.projection {
self.recordbatch
Expand Down
24 changes: 12 additions & 12 deletions tests-integration/src/tests/instance_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,21 +1422,21 @@ async fn test_information_schema_dot_tables(instance: Arc<dyn MockInstance>) {
let expected = match is_distributed_mode {
true => {
"\
+---------------+--------------+------------+------------+----------+-------------+
| table_catalog | table_schema | table_name | table_type | table_id | engine |
+---------------+--------------+------------+------------+----------+-------------+
| greptime | public | numbers | BASE TABLE | 2 | test_engine |
| greptime | public | scripts | BASE TABLE | 1024 | mito |
+---------------+--------------+------------+------------+----------+-------------+"
+---------------+--------------+------------+-----------------+----------+-------------+
| table_catalog | table_schema | table_name | table_type | table_id | engine |
+---------------+--------------+------------+-----------------+----------+-------------+
| greptime | public | numbers | LOCAL TEMPORARY | 2 | test_engine |
| greptime | public | scripts | BASE TABLE | 1024 | mito |
+---------------+--------------+------------+-----------------+----------+-------------+"
}
false => {
"\
+---------------+--------------+------------+------------+----------+-------------+
| table_catalog | table_schema | table_name | table_type | table_id | engine |
+---------------+--------------+------------+------------+----------+-------------+
| greptime | public | numbers | BASE TABLE | 2 | test_engine |
| greptime | public | scripts | BASE TABLE | 1 | mito |
+---------------+--------------+------------+------------+----------+-------------+"
+---------------+--------------+------------+-----------------+----------+-------------+
| table_catalog | table_schema | table_name | table_type | table_id | engine |
+---------------+--------------+------------+-----------------+----------+-------------+
| greptime | public | numbers | LOCAL TEMPORARY | 2 | test_engine |
| greptime | public | scripts | BASE TABLE | 1 | mito |
+---------------+--------------+------------+-----------------+----------+-------------+"
}
};

Expand Down
10 changes: 5 additions & 5 deletions tests/cases/distributed/optimizer/order_by.result
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ explain select * from numbers;
+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| plan_type | plan |
+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | MergeScan [is_placeholder=false] |
| logical_plan | TableScan: numbers projection=[number] |
| physical_plan | StreamScanAdapter { stream: "<SendableRecordBatchStream>", schema: [Field { name: "number", data_type: UInt32, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }] } |
| | |
+---------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
Expand All @@ -14,7 +14,7 @@ explain select * from numbers order by number desc;
| plan_type | plan |
+---------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | Sort: numbers.number DESC NULLS FIRST |
| | MergeScan [is_placeholder=false] |
| | TableScan: numbers projection=[number] |
| physical_plan | SortExec: expr=[number@0 DESC] |
| | StreamScanAdapter { stream: "<SendableRecordBatchStream>", schema: [Field { name: "number", data_type: UInt32, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }] } |
| | |
Expand All @@ -26,7 +26,7 @@ explain select * from numbers order by number asc;
| plan_type | plan |
+---------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | Sort: numbers.number ASC NULLS LAST |
| | MergeScan [is_placeholder=false] |
| | TableScan: numbers projection=[number] |
| physical_plan | SortExec: expr=[number@0 ASC NULLS LAST] |
| | StreamScanAdapter { stream: "<SendableRecordBatchStream>", schema: [Field { name: "number", data_type: UInt32, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }] } |
| | |
Expand All @@ -39,7 +39,7 @@ explain select * from numbers order by number desc limit 10;
+---------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | Limit: skip=0, fetch=10 |
| | Sort: numbers.number DESC NULLS FIRST, fetch=10 |
| | MergeScan [is_placeholder=false] |
| | TableScan: numbers projection=[number] |
| physical_plan | GlobalLimitExec: skip=0, fetch=10 |
| | SortExec: fetch=10, expr=[number@0 DESC] |
| | StreamScanAdapter { stream: "<SendableRecordBatchStream>", schema: [Field { name: "number", data_type: UInt32, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }] } |
Expand All @@ -53,7 +53,7 @@ explain select * from numbers order by number asc limit 10;
+---------------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| logical_plan | Limit: skip=0, fetch=10 |
| | Sort: numbers.number ASC NULLS LAST, fetch=10 |
| | MergeScan [is_placeholder=false] |
| | TableScan: numbers projection=[number] |
| physical_plan | GlobalLimitExec: skip=0, fetch=10 |
| | SortExec: fetch=10, expr=[number@0 ASC NULLS LAST] |
| | StreamScanAdapter { stream: "<SendableRecordBatchStream>", schema: [Field { name: "number", data_type: UInt32, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }] } |
Expand Down

0 comments on commit 7abe71f

Please sign in to comment.