mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-10-15 16:32:34 +00:00
fix(table): stop claiming tag order for logical metric scans (#9522)
* test: cover metric logical scan tag ordering Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(table): only claim projected primary key prefix ordering Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(table): keep physical key ordering for metric projections Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(table): compare unknown partition counts explicitly Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(table): model scoped internal TSID scan properties Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
This commit is contained in:
discord9
+265
-7
@@ -118,21 +118,34 @@ impl RegionScanExec {
|
||||
}
|
||||
|
||||
let metadata = scanner.metadata();
|
||||
let mut pk_names = metadata
|
||||
// Keep the primary key columns in the order reported by the metadata (for a metric
|
||||
// region that is the physical row key order). Reordering them would let a projected
|
||||
// logical tag look like a prefix of the row key, which yields ordering and
|
||||
// partitioning claims the scanner cannot honour.
|
||||
let pk_names = metadata
|
||||
.primary_key_columns()
|
||||
.map(|col| col.column_schema.name.clone())
|
||||
.collect::<Vec<_>>();
|
||||
// workaround for logical table
|
||||
if scanner.properties().is_logical_region() {
|
||||
pk_names.sort_unstable();
|
||||
}
|
||||
// A region emits rows ordered by its primary key columns and then by the time
|
||||
// index column. A primary key column that is not projected is absent from the
|
||||
// arrow schema, and it also breaks the ordering of every primary key column that
|
||||
// follows it (e.g. the physical `__table_id`/`__tsid` prefix of a metric region
|
||||
// when reading a logical table). Thus only the longest fully projected prefix of
|
||||
// the primary key can describe the output ordering, and that prefix can be empty.
|
||||
let projected_pk_prefix_len = pk_names
|
||||
.iter()
|
||||
.position(|name| Column::new_with_schema(name, &arrow_schema).is_err())
|
||||
.unwrap_or(pk_names.len());
|
||||
// Whether the projected prefix covers the whole row key.
|
||||
let all_pk_projected = projected_pk_prefix_len == pk_names.len();
|
||||
let projected_pk_names = &pk_names[..projected_pk_prefix_len];
|
||||
let pk_columns = pk_names
|
||||
.iter()
|
||||
.filter_map(
|
||||
|col| Some(Arc::new(Column::new_with_schema(col, &arrow_schema).ok()?) as _),
|
||||
)
|
||||
.collect::<Vec<_>>();
|
||||
let pk_sort_columns: Vec<PhysicalSortExpr> = pk_names
|
||||
let pk_sort_columns: Vec<PhysicalSortExpr> = projected_pk_names
|
||||
.iter()
|
||||
.filter_map(|col| {
|
||||
Some(PhysicalSortExpr::new(
|
||||
@@ -165,7 +178,9 @@ impl RegionScanExec {
|
||||
let mut orderings = Vec::with_capacity(2);
|
||||
|
||||
let mut pk_time_ordering = pk_sort_columns.clone();
|
||||
if let Some(ts) = ts_col.clone() {
|
||||
// The time index column extends the ordering only when the whole row key
|
||||
// precedes it; otherwise rows of different series can interleave.
|
||||
if all_pk_projected && let Some(ts) = ts_col.clone() {
|
||||
pk_time_ordering.push(ts);
|
||||
}
|
||||
orderings.push(pk_time_ordering);
|
||||
@@ -694,9 +709,11 @@ mod test {
|
||||
use datatypes::vectors::{Int32Vector, TimestampMillisecondVector, VectorRef};
|
||||
use futures::TryStreamExt;
|
||||
use store_api::metadata::{ColumnMetadata, RegionMetadataBuilder};
|
||||
use store_api::metric_engine_consts::DATA_SCHEMA_TABLE_ID_COLUMN_NAME;
|
||||
use store_api::region_engine::{
|
||||
PrepareRequest, QueryScanContext, RegionScanner, ScannerProperties, SinglePartitionScanner,
|
||||
};
|
||||
use store_api::storage::consts::ReservedColumnId;
|
||||
use store_api::storage::{RegionId, TimeSeriesDistribution};
|
||||
|
||||
use super::*;
|
||||
@@ -953,6 +970,247 @@ mod test {
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns whether the scan claims the given output ordering.
|
||||
fn claims_ordering(plan: &RegionScanExec, names: &[&str]) -> bool {
|
||||
plan.properties()
|
||||
.equivalence_properties()
|
||||
.ordering_satisfy(names.iter().map(|name| {
|
||||
PhysicalSortExpr::new(
|
||||
Arc::new(Column::new_with_schema(name, &plan.arrow_schema).unwrap()),
|
||||
SortOptions {
|
||||
descending: false,
|
||||
nulls_first: true,
|
||||
},
|
||||
)
|
||||
}))
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
/// Column schema used by the ordering fixtures below.
|
||||
fn column_schema_of(name: &str) -> ColumnSchema {
|
||||
let data_type = match name {
|
||||
"ts" => ConcreteDataType::timestamp_millisecond_datatype(),
|
||||
DATA_SCHEMA_TABLE_ID_COLUMN_NAME => ConcreteDataType::uint32_datatype(),
|
||||
DATA_SCHEMA_TSID_COLUMN_NAME => ConcreteDataType::uint64_datatype(),
|
||||
_ => ConcreteDataType::string_datatype(),
|
||||
};
|
||||
ColumnSchema::new(name, data_type, false)
|
||||
}
|
||||
|
||||
/// The scan must describe its output ordering with the primary key order reported by
|
||||
/// the metadata. Sorting the names (the removed logical-table workaround) would let a
|
||||
/// projected column look like a row key prefix and claim an ordering the scanner does
|
||||
/// not produce; the time index only extends a fully projected row key.
|
||||
#[test]
|
||||
fn test_per_series_ordering_follows_metadata_pk_order() {
|
||||
// Deliberately not alphabetical: sorting these names would give `[a, b, z]`.
|
||||
let mut builder = RegionMetadataBuilder::new(RegionId::new(1234, 5690));
|
||||
for (index, name) in ["z", "a", "b", "ts"].into_iter().enumerate() {
|
||||
builder.push_column_metadata(ColumnMetadata {
|
||||
column_schema: column_schema_of(name),
|
||||
semantic_type: if name == "ts" {
|
||||
SemanticType::Timestamp
|
||||
} else {
|
||||
SemanticType::Tag
|
||||
},
|
||||
column_id: index as u32 + 1,
|
||||
});
|
||||
}
|
||||
builder.primary_key(vec![1, 2, 3]);
|
||||
let metadata = Arc::new(builder.build().unwrap());
|
||||
|
||||
// The claims must not depend on the scanner reporting a logical region.
|
||||
for logical_region in [false, true] {
|
||||
for (projection, claimed, unclaimed) in [
|
||||
// Missing the first key: neither the remaining keys nor the time index are
|
||||
// ordered.
|
||||
(
|
||||
vec!["a", "b", "ts"],
|
||||
vec![],
|
||||
vec![vec!["a"], vec!["b"], vec!["ts"], vec!["a", "b"]],
|
||||
),
|
||||
// Missing a key in the middle: only the prefix before it is ordered.
|
||||
(
|
||||
vec!["z", "b", "ts"],
|
||||
vec![vec!["z"]],
|
||||
vec![vec!["b"], vec!["z", "b"], vec!["z", "ts"]],
|
||||
),
|
||||
// A valid but incomplete prefix: the time index is not ordered after it.
|
||||
(
|
||||
vec!["z", "a", "ts"],
|
||||
vec![vec!["z"], vec!["z", "a"]],
|
||||
vec![vec!["a", "z"], vec!["z", "a", "ts"]],
|
||||
),
|
||||
// The whole row key followed by the time index.
|
||||
(
|
||||
vec!["z", "a", "b", "ts"],
|
||||
vec![vec!["z", "a", "b"], vec!["z", "a", "b", "ts"]],
|
||||
vec![vec!["a"], vec!["a", "b", "z"]],
|
||||
),
|
||||
] {
|
||||
let schema = Arc::new(Schema::new(
|
||||
projection
|
||||
.iter()
|
||||
.map(|name| column_schema_of(name))
|
||||
.collect(),
|
||||
));
|
||||
let mut properties = ScannerProperties::new(vec![vec![], vec![]], false, 0);
|
||||
properties.set_logical_region(logical_region);
|
||||
let scanner = Box::new(RepeatableScanner {
|
||||
batches: RecordBatches::try_new(schema, vec![]).unwrap(),
|
||||
properties,
|
||||
metadata: metadata.clone(),
|
||||
dynamic_filters: Arc::new(Mutex::new(Vec::new())),
|
||||
});
|
||||
let plan = RegionScanExec::new(
|
||||
scanner,
|
||||
ScanRequest {
|
||||
distribution: Some(TimeSeriesDistribution::PerSeries),
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
for ordering in claimed {
|
||||
assert!(
|
||||
claims_ordering(&plan, &ordering),
|
||||
"logical_region={logical_region} projection {projection:?} must claim \
|
||||
ordering {ordering:?}"
|
||||
);
|
||||
}
|
||||
for ordering in unclaimed {
|
||||
assert!(
|
||||
!claims_ordering(&plan, &ordering),
|
||||
"logical_region={logical_region} projection {projection:?} must not \
|
||||
claim ordering {ordering:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Metric TSID properties require projecting `__tsid` and restricting the scan to one
|
||||
/// `__table_id`; hiding the physical row-key prefix must not claim a visible tag or time as
|
||||
/// order or hash partition columns.
|
||||
#[test]
|
||||
fn test_metric_projection_claims_only_projected_row_key_prefix() {
|
||||
let mut builder = RegionMetadataBuilder::new(RegionId::new(1234, 5691));
|
||||
for (name, column_id) in [
|
||||
(
|
||||
DATA_SCHEMA_TABLE_ID_COLUMN_NAME,
|
||||
ReservedColumnId::table_id(),
|
||||
),
|
||||
(DATA_SCHEMA_TSID_COLUMN_NAME, ReservedColumnId::tsid()),
|
||||
("A", 1),
|
||||
("marker", 2),
|
||||
("ts", 3),
|
||||
] {
|
||||
builder.push_column_metadata(ColumnMetadata {
|
||||
column_schema: column_schema_of(name),
|
||||
semantic_type: if name == "ts" {
|
||||
SemanticType::Timestamp
|
||||
} else {
|
||||
SemanticType::Tag
|
||||
},
|
||||
column_id,
|
||||
});
|
||||
}
|
||||
// The physical row key order: `__table_id`, `__tsid`, then the tag columns.
|
||||
builder.primary_key(vec![
|
||||
ReservedColumnId::table_id(),
|
||||
ReservedColumnId::tsid(),
|
||||
1,
|
||||
2,
|
||||
]);
|
||||
let metadata = Arc::new(builder.build().unwrap());
|
||||
|
||||
for (projection, partitioned_by_tsid, claimed, unclaimed) in [
|
||||
// The logical projection drops the row key prefix, so `A` is not a prefix of the
|
||||
// row key and neither `A` nor `ts` is ordered.
|
||||
(
|
||||
vec!["A", "marker", "ts"],
|
||||
false,
|
||||
vec![],
|
||||
vec![vec!["A"], vec!["marker"], vec!["ts"], vec!["A", "ts"]],
|
||||
),
|
||||
// Projecting internal `__tsid` plus the single-table filter preserves metric TSID properties.
|
||||
(
|
||||
vec![DATA_SCHEMA_TSID_COLUMN_NAME, "A", "marker", "ts"],
|
||||
true,
|
||||
vec![
|
||||
vec![DATA_SCHEMA_TSID_COLUMN_NAME],
|
||||
vec![DATA_SCHEMA_TSID_COLUMN_NAME, "ts"],
|
||||
],
|
||||
vec![vec!["A"], vec!["ts"], vec!["A", "ts"]],
|
||||
),
|
||||
] {
|
||||
let schema = Arc::new(Schema::new(
|
||||
projection
|
||||
.iter()
|
||||
.map(|name| column_schema_of(name))
|
||||
.collect(),
|
||||
));
|
||||
let mut properties = ScannerProperties::new(vec![vec![], vec![]], false, 0);
|
||||
properties.set_logical_region(true);
|
||||
let scanner = Box::new(RepeatableScanner {
|
||||
batches: RecordBatches::try_new(schema, vec![]).unwrap(),
|
||||
properties,
|
||||
metadata: metadata.clone(),
|
||||
dynamic_filters: Arc::new(Mutex::new(Vec::new())),
|
||||
});
|
||||
let plan = RegionScanExec::new(
|
||||
scanner,
|
||||
ScanRequest {
|
||||
distribution: Some(TimeSeriesDistribution::PerSeries),
|
||||
filters: if partitioned_by_tsid {
|
||||
vec![
|
||||
datafusion_expr::col(DATA_SCHEMA_TABLE_ID_COLUMN_NAME)
|
||||
.eq(datafusion_expr::lit(42u32)),
|
||||
]
|
||||
} else {
|
||||
vec![]
|
||||
},
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let expected_partitioning = if partitioned_by_tsid {
|
||||
Partitioning::Hash(
|
||||
vec![Arc::new(
|
||||
Column::new_with_schema(DATA_SCHEMA_TSID_COLUMN_NAME, &plan.arrow_schema)
|
||||
.unwrap(),
|
||||
) as _],
|
||||
2,
|
||||
)
|
||||
} else {
|
||||
Partitioning::UnknownPartitioning(2)
|
||||
};
|
||||
match (&plan.properties.partitioning, &expected_partitioning) {
|
||||
(
|
||||
Partitioning::UnknownPartitioning(actual_count),
|
||||
Partitioning::UnknownPartitioning(expected_count),
|
||||
) => assert_eq!(actual_count, expected_count, "projection {projection:?}"),
|
||||
(actual, expected) => assert_eq!(actual, expected, "projection {projection:?}"),
|
||||
}
|
||||
|
||||
for ordering in claimed {
|
||||
assert!(
|
||||
claims_ordering(&plan, &ordering),
|
||||
"projection {projection:?} must claim ordering {ordering:?}"
|
||||
);
|
||||
}
|
||||
for ordering in unclaimed {
|
||||
assert!(
|
||||
!claims_ordering(&plan, &ordering),
|
||||
"projection {projection:?} must not claim ordering {ordering:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_count_statistics_require_exact_source_rows() {
|
||||
for (append_mode, exact, expected) in [
|
||||
|
||||
Reference in New Issue
Block a user