Skip to content

Commit

Permalink
chore(cubestore): Upgrade DF: Use lowercase names for UDAF registry
Browse files Browse the repository at this point in the history
  • Loading branch information
srh committed Dec 3, 2024
1 parent d84d4a4 commit 1500d69
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
10 changes: 5 additions & 5 deletions rust/cubestore/cubestore/src/queryplanner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,18 +502,18 @@ impl ContextProvider for MetaStoreSchemaProvider {
return Some(scalar_udf_by_kind(kind));
}

fn get_aggregate_meta(&self, name: &str) -> Option<Arc<AggregateUDF>> {
fn get_aggregate_meta(&self, name_param: &str) -> Option<Arc<AggregateUDF>> {
// HyperLogLog.
// TODO: case-insensitive names.
/*
let (_kind, name) = match name {
"merge" | "MERGE" => (CubeAggregateUDFKind::MergeHll, "MERGE"),
_ => return None,
};
*/

Check warning on line 513 in rust/cubestore/cubestore/src/queryplanner/mod.rs

View workflow job for this annotation

GitHub Actions / Debian Rust nightly-2024-01-29

Diff in /home/runner/work/cube/cube/rust/cubestore/cubestore/src/queryplanner/mod.rs
let name = name_param.to_ascii_lowercase();

let aggregate_udf_by_registry = self.session_state.aggregate_functions().get(name);

// TODO upgrade DF: Remove this assertion (and/or remove the kind lookup above).
assert!(aggregate_udf_by_registry.is_some(), "MERGE is not registered in SessionState");
let aggregate_udf_by_registry: Option<&Arc<AggregateUDF>> = self.session_state.aggregate_functions().get(&name);

aggregate_udf_by_registry.map(|arc| arc.clone())
}
Expand Down
8 changes: 6 additions & 2 deletions rust/cubestore/cubestore/src/queryplanner/udfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ pub fn scalar_kind_by_name(n: &str) -> Option<CubeScalarUDFKind> {
if n == "DATE_BIN" {
return Some(CubeScalarUDFKind::DateBin);

Check warning on line 79 in rust/cubestore/cubestore/src/queryplanner/udfs.rs

View workflow job for this annotation

GitHub Actions / Debian Rust nightly-2024-01-29

Diff in /home/runner/work/cube/cube/rust/cubestore/cubestore/src/queryplanner/udfs.rs
}
// TODO upgrade DF: Remove this (once we are no longer in flux about naming casing of UDFs and UDAFs).
if ["CARDINALITY", /* "COALESCE", "NOW", */ "UNIX_TIMESTAMP", "DATE_ADD", "DATE_SUB", "DATE_BIN"].contains(&(&n.to_ascii_uppercase() as &str)) {
panic!("scalar_kind_by_name failing on '{}' due to uppercase/lowercase mixup", n);
}
return None;
}

Expand Down Expand Up @@ -109,7 +113,7 @@ pub fn aggregate_udf_by_kind(k: CubeAggregateUDFKind) -> AggregateUDF {

/// Note that only full match counts. Pass capitalized names.
pub fn aggregate_kind_by_name(n: &str) -> Option<CubeAggregateUDFKind> {
if n == "MERGE" {
if n == "merge" {
return Some(CubeAggregateUDFKind::MergeHll);
}
return None;

Check warning on line 119 in rust/cubestore/cubestore/src/queryplanner/udfs.rs

View workflow job for this annotation

GitHub Actions / Debian Rust nightly-2024-01-29

Diff in /home/runner/work/cube/cube/rust/cubestore/cubestore/src/queryplanner/udfs.rs
Expand Down Expand Up @@ -642,7 +646,7 @@ impl HllMergeUDF {
impl AggregateUDFImpl for HllMergeUDF {

fn name(&self) -> &str {
return "MERGE";
return "merge";
}

fn as_any(&self) -> &dyn Any {
Expand Down

0 comments on commit 1500d69

Please sign in to comment.