fix: honor default prefix for all metric columns (#8640)

* fix: honor default prefix for metric columns

Signed-off-by: shuiyisong <xixing.sys@gmail.com>

* fix: cr issue

Signed-off-by: shuiyisong <xixing.sys@gmail.com>

---------

Signed-off-by: shuiyisong <xixing.sys@gmail.com>
This commit is contained in:
shuiyisong
2026-07-27 07:35:28 +00:00
committed by GitHub
parent f5f5d468bb
commit b462d5d19e
19 changed files with 189 additions and 136 deletions
+26 -72
View File
@@ -22,8 +22,7 @@ use arrow_schema::DataType;
use arrow_schema::extension::{EXTENSION_TYPE_NAME_KEY, ExtensionType};
use common_base::readable_size::ReadableSize;
use common_query::native_histogram::{
is_native_histogram_value_schema, native_histogram_list_element_id,
native_histogram_subfield_id,
is_native_histogram_value_type, native_histogram_list_element_id, native_histogram_subfield_id,
};
use datatypes::arrow::datatypes::{
DataType as ArrowDataType, Field, FieldRef, Fields, Schema, SchemaRef,
@@ -87,10 +86,10 @@ pub fn with_field_id(mut field: Field, column_id: u32) -> Field {
/// fields), so external readers can identify it by extension and resolve
/// nested fields by id.
///
/// Detection is by the native-histogram column name and struct type
/// (`is_native_histogram_value_schema`); other struct columns are left
/// untouched. mito2 reads SST columns by schema position, never by field
/// metadata, so this only affects external readers.
/// Detection is by the exact native-histogram struct type
/// (`is_native_histogram_value_type`); other struct columns are left untouched.
/// mito2 reads SST columns by schema position, never by field metadata, so this
/// only affects external readers.
///
/// Returns an error if the parent column's `PARQUET:field_id` is missing,
/// malformed, or exceeds `i32::MAX`, or if a sub-field id cannot be derived
@@ -98,10 +97,7 @@ pub fn with_field_id(mut field: Field, column_id: u32) -> Field {
/// the derived id overflows a positive `i32` (an absurdly large parent
/// `column_id`); see [`native_histogram_subfield_id`].
fn stamp_native_histogram_subfield_ids(field: &mut Field) -> crate::error::Result<()> {
if !is_native_histogram_value_schema(
field.name(),
&ConcreteDataType::from_arrow_type(field.data_type()),
) {
if !is_native_histogram_value_type(&ConcreteDataType::from_arrow_type(field.data_type())) {
return Ok(());
}
// Namespace sub-field ids by the parent column's field id (its
@@ -509,6 +505,7 @@ impl SeriesEstimator {
mod tests {
use std::sync::Arc;
use common_query::prelude::greptime_native_histogram;
use datatypes::arrow::array::{
BinaryArray, DictionaryArray, TimestampMillisecondArray, UInt8Array, UInt32Array,
UInt64Array,
@@ -750,20 +747,18 @@ mod tests {
#[test]
fn test_maybe_wrap_schema_native_histogram() {
use common_query::native_histogram::NATIVE_HISTOGRAM_FIELD;
let schema = Arc::new(Schema::new(vec![
Field::new(
"greptime_timestamp",
ArrowDataType::Timestamp(TimeUnit::Millisecond, None),
false,
),
histogram_field(NATIVE_HISTOGRAM_FIELD, 1),
histogram_field(greptime_native_histogram(), 1),
]));
let wrapped = maybe_wrap_schema(&schema).unwrap();
let hist = wrapped
.field_with_name(NATIVE_HISTOGRAM_FIELD)
.field_with_name(greptime_native_histogram())
.expect("histogram field present");
// The struct has 18 sub-fields.
let ArrowDataType::Struct(children) = hist.data_type() else {
@@ -778,13 +773,11 @@ mod tests {
// Two histogram columns with distinct parent column ids get disjoint
// sub-field ids (defensive: the metric engine yields at most one
// histogram column, but the scheme must stay correct if more appear).
use common_query::native_histogram::{
NATIVE_HISTOGRAM_FIELD, native_histogram_subfield_id,
};
use common_query::native_histogram::native_histogram_subfield_id;
let schema = Arc::new(Schema::new(vec![
histogram_field(NATIVE_HISTOGRAM_FIELD, 1),
histogram_field(NATIVE_HISTOGRAM_FIELD, 7),
histogram_field(greptime_native_histogram(), 1),
histogram_field(greptime_native_histogram(), 7),
]));
let wrapped = maybe_wrap_schema(&schema).unwrap();
let h1 = &wrapped.fields()[0];
@@ -799,26 +792,12 @@ mod tests {
}
#[test]
fn test_maybe_wrap_schema_requires_canonical_name() {
// Detection requires the canonical column name: a histogram-typed field
// named differently is left untouched.
use arrow_schema::extension::EXTENSION_TYPE_NAME_KEY;
use common_query::native_histogram::native_histogram_value_type;
use datatypes::data_type::DataType;
let hist_arrow = native_histogram_value_type().as_arrow_type();
let schema = Arc::new(Schema::new(vec![Field::new(
"custom_histogram",
hist_arrow,
true,
)]));
fn test_maybe_wrap_schema_recognizes_histogram_by_type() {
let schema = Arc::new(Schema::new(vec![histogram_field("custom_histogram", 5)]));
let wrapped = maybe_wrap_schema(&schema).unwrap();
let hist = wrapped.field_with_name("custom_histogram").unwrap();
assert!(
hist.metadata().get(EXTENSION_TYPE_NAME_KEY).is_none(),
"a histogram-typed field without the canonical name must not be stamped"
);
assert_histogram_stamped(hist, 5);
}
#[test]
@@ -911,65 +890,42 @@ mod tests {
// On-disk contract: after writing through the parquet writer path, the
// footer still carries the greptime.histogram extension and every
// nested (sub-field + list-element) PARQUET:field_id.
use common_query::native_histogram::NATIVE_HISTOGRAM_FIELD;
let schema = Arc::new(Schema::new(vec![
Field::new(
"greptime_timestamp",
ArrowDataType::Timestamp(TimeUnit::Millisecond, None),
false,
),
histogram_field(NATIVE_HISTOGRAM_FIELD, 3),
histogram_field(greptime_native_histogram(), 3),
]));
let on_disk = parquet_footer_arrow_schema(&schema);
let hist = on_disk
.field_with_name(NATIVE_HISTOGRAM_FIELD)
.field_with_name(greptime_native_histogram())
.expect("histogram field present");
assert_histogram_stamped(hist, 3);
}
#[test]
fn test_parquet_roundtrip_noncanonical_struct_untouched() {
// A histogram-shaped struct without the canonical column name is left
// untouched on disk: no extension, no nested field ids.
use arrow_schema::extension::EXTENSION_TYPE_NAME_KEY;
use common_query::native_histogram::native_histogram_value_type;
use datatypes::data_type::DataType;
let hist_arrow = native_histogram_value_type().as_arrow_type();
fn test_parquet_roundtrip_recognizes_histogram_by_type() {
// The persisted type, rather than a process-local configured name,
// identifies native histograms across upgrades and prefix changes.
let schema = Arc::new(Schema::new(vec![
Field::new(
"ts",
ArrowDataType::Timestamp(TimeUnit::Millisecond, None),
false,
),
Field::new("custom_histogram", hist_arrow, true),
histogram_field("custom_histogram", 9),
]));
let on_disk = parquet_footer_arrow_schema(&schema);
let hist = on_disk.field_with_name("custom_histogram").unwrap();
assert!(
hist.metadata().get(EXTENSION_TYPE_NAME_KEY).is_none(),
"a histogram-typed field without the canonical name must not be stamped on disk"
);
if let ArrowDataType::Struct(children) = hist.data_type() {
for child in children {
assert!(
child.metadata().get(PARQUET_FIELD_ID_KEY).is_none(),
"non-histogram sub-field {} must not get a field id on disk",
child.name()
);
}
} else {
panic!("expected a struct, got {:?}", hist.data_type());
}
assert_histogram_stamped(hist, 9);
}
#[test]
fn test_maybe_wrap_schema_overflows_return_error() {
use common_query::native_histogram::NATIVE_HISTOGRAM_FIELD;
// A column id of 12_582_912 makes the derived sub-field id overflow
// i32 (BASE + column_id*64 == i32::MAX + 1). The write path must
// surface this as an error rather than silently dropping the field
@@ -980,7 +936,7 @@ mod tests {
ArrowDataType::Timestamp(TimeUnit::Millisecond, None),
false,
),
histogram_field(NATIVE_HISTOGRAM_FIELD, 12_582_912),
histogram_field(greptime_native_histogram(), 12_582_912),
]));
let err = maybe_wrap_schema(&schema).unwrap_err();
assert!(
@@ -1000,11 +956,11 @@ mod tests {
// without the write path's stamping), the writer must fail loudly
// rather than silently namespace under column 0, which would collide
// with that column's nested ids.
use common_query::native_histogram::{NATIVE_HISTOGRAM_FIELD, native_histogram_value_type};
use common_query::native_histogram::native_histogram_value_type;
use datatypes::data_type::DataType;
let field = Field::new(
NATIVE_HISTOGRAM_FIELD,
greptime_native_histogram(),
native_histogram_value_type().as_arrow_type(),
true,
);
@@ -1030,15 +986,13 @@ mod tests {
// above i32::MAX (e.g. u32::MAX) must not be silently parsed as a
// failed i32 and collapsed onto column 0's nested ids. It must
// surface a checked-conversion error instead.
use common_query::native_histogram::NATIVE_HISTOGRAM_FIELD;
let schema = Arc::new(Schema::new(vec![
Field::new(
"greptime_timestamp",
ArrowDataType::Timestamp(TimeUnit::Millisecond, None),
false,
),
histogram_field(NATIVE_HISTOGRAM_FIELD, u32::MAX),
histogram_field(greptime_native_histogram(), u32::MAX),
]));
let err = maybe_wrap_schema(&schema).unwrap_err();
assert!(