Skip to content

Commit dd03903

Browse files
committed
Make dataset alias comparisons case-insensitive
1 parent 3dac0f2 commit dd03903

File tree

4 files changed

+57
-10
lines changed

4 files changed

+57
-10
lines changed

src/domain/opendatafabric/src/identity/dataset_identity.rs

+43-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// by the Apache License, Version 2.0.
99

1010
use std::convert::{AsRef, TryFrom};
11+
use std::hash::Hash;
1112
use std::sync::Arc;
1213
use std::{cmp, fmt, ops};
1314

@@ -104,7 +105,7 @@ macro_rules! impl_serde {
104105
}
105106

106107
pub(crate) use impl_serde;
107-
use like::Like;
108+
use like::ILike;
108109

109110
////////////////////////////////////////////////////////////////////////////////
110111

@@ -233,7 +234,7 @@ newtype_str!(
233234

234235
impl DatasetNamePattern {
235236
pub fn matches(&self, dataset_name: &DatasetName) -> bool {
236-
Like::<false>::like(dataset_name.as_str(), self).unwrap()
237+
ILike::<false>::ilike(dataset_name.as_str(), self).unwrap()
237238
}
238239
}
239240

@@ -311,7 +312,7 @@ newtype_str!(RepoName, Grammar::match_repo_name, RepoNameSerdeVisitor);
311312

312313
////////////////////////////////////////////////////////////////////////////////
313314

314-
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
315+
#[derive(Debug, Clone, Eq, PartialOrd, Ord)]
315316
pub struct DatasetAlias {
316317
pub account_name: Option<AccountName>,
317318
pub dataset_name: DatasetName,
@@ -358,6 +359,24 @@ impl DatasetAlias {
358359
}
359360
}
360361

362+
impl PartialEq<Self> for DatasetAlias {
363+
fn eq(&self, other: &Self) -> bool {
364+
((self.account_name.is_some()
365+
&& other.account_name.is_some()
366+
&& self.account_name.as_ref().unwrap().to_lowercase()
367+
== other.account_name.as_ref().unwrap().to_lowercase())
368+
|| (self.account_name.is_none() && other.account_name.is_none()))
369+
&& self.dataset_name.to_lowercase() == other.dataset_name.to_lowercase()
370+
}
371+
}
372+
373+
impl Hash for DatasetAlias {
374+
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
375+
self.account_name.hash(state);
376+
self.dataset_name.hash(state);
377+
}
378+
}
379+
361380
impl std::str::FromStr for DatasetAlias {
362381
type Err = ParseError<Self>;
363382
fn from_str(s: &str) -> Result<Self, Self::Err> {
@@ -388,7 +407,7 @@ impl_serde!(DatasetAlias, DatasetAliasSerdeVisitor);
388407

389408
////////////////////////////////////////////////////////////////////////////////
390409

391-
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
410+
#[derive(Debug, Clone, Eq, PartialOrd, Ord)]
392411
pub struct DatasetAliasRemote {
393412
pub repo_name: RepoName,
394413
pub account_name: Option<AccountName>,
@@ -437,6 +456,26 @@ impl DatasetAliasRemote {
437456
}
438457
}
439458

459+
impl PartialEq<Self> for DatasetAliasRemote {
460+
fn eq(&self, other: &Self) -> bool {
461+
(self.repo_name.to_lowercase() == other.repo_name.to_lowercase())
462+
&& ((self.account_name.is_some()
463+
&& other.account_name.is_some()
464+
&& self.account_name.as_ref().unwrap().to_lowercase()
465+
== other.account_name.as_ref().unwrap().to_lowercase())
466+
|| (self.account_name.is_none() && other.account_name.is_none()))
467+
&& self.dataset_name.to_lowercase() == other.dataset_name.to_lowercase()
468+
}
469+
}
470+
471+
impl Hash for DatasetAliasRemote {
472+
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
473+
self.repo_name.hash(state);
474+
self.account_name.hash(state);
475+
self.dataset_name.hash(state);
476+
}
477+
}
478+
440479
impl std::str::FromStr for DatasetAliasRemote {
441480
type Err = ParseError<Self>;
442481
fn from_str(s: &str) -> Result<Self, Self::Err> {

src/infra/core/src/repos/dataset_repository_local_fs.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ impl DatasetStorageStrategy for DatasetSingleTenantStorageStrategy {
516516
}
517517

518518
fn get_datasets_by_owner(&self, account_name: AccountName) -> DatasetHandleStream<'_> {
519-
if account_name == DEFAULT_ACCOUNT_NAME {
519+
if account_name.to_lowercase() == DEFAULT_ACCOUNT_NAME {
520520
self.get_all_datasets()
521521
} else {
522522
Box::pin(futures::stream::empty())
@@ -529,7 +529,8 @@ impl DatasetStorageStrategy for DatasetSingleTenantStorageStrategy {
529529
) -> Result<DatasetHandle, ResolveDatasetError> {
530530
assert!(
531531
!dataset_alias.is_multi_tenant()
532-
|| dataset_alias.account_name.as_ref().unwrap() == DEFAULT_ACCOUNT_NAME,
532+
|| dataset_alias.account_name.as_ref().unwrap().to_lowercase()
533+
== DEFAULT_ACCOUNT_NAME,
533534
"Multi-tenant refs shouldn't have reached down to here with earlier validations"
534535
);
535536

@@ -809,7 +810,9 @@ impl DatasetStorageStrategy for DatasetMultiTenantStorageStrategy {
809810
)
810811
.await?;
811812

812-
if candidate_dataset_alias.dataset_name == dataset_alias.dataset_name {
813+
if candidate_dataset_alias.dataset_name.to_lowercase()
814+
== dataset_alias.dataset_name.to_lowercase()
815+
{
813816
return Ok(DatasetHandle::new(dataset_id, candidate_dataset_alias));
814817
}
815818
}

src/infra/core/src/repos/dataset_repository_s3.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,13 @@ impl DatasetRepository for DatasetRepositoryS3 {
197197
}
198198

199199
fn get_datasets_by_owner(&self, account_name: AccountName) -> DatasetHandleStream<'_> {
200-
if !self.is_multi_tenant() && account_name != DEFAULT_ACCOUNT_NAME {
200+
if !self.is_multi_tenant() && account_name.to_lowercase() != DEFAULT_ACCOUNT_NAME {
201201
return Box::pin(futures::stream::empty());
202202
}
203203

204204
self.stream_datasets_if(move |dataset_alias| {
205205
if let Some(dataset_account_name) = &dataset_alias.account_name {
206-
dataset_account_name == &account_name
206+
dataset_account_name.to_lowercase() == account_name.to_lowercase()
207207
} else {
208208
true
209209
}

src/infra/core/src/utils/datasets_filtering.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,12 @@ pub fn matches_remote_ref_pattern(
161161
DatasetRefAnyPattern::PatternRemote(repo_name, account_name, dataset_name_pattern) => {
162162
repo_name == &dataset_alias_remote.repo_name
163163
&& (dataset_alias_remote.account_name.is_some()
164-
&& account_name == dataset_alias_remote.account_name.as_ref().unwrap())
164+
&& account_name.to_lowercase()
165+
== dataset_alias_remote
166+
.account_name
167+
.as_ref()
168+
.unwrap()
169+
.to_lowercase())
165170
&& dataset_name_pattern.matches(&dataset_alias_remote.dataset_name)
166171
}
167172
}

0 commit comments

Comments
 (0)