Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Fix 'filterable' info in field schema #11418

Merged
merged 4 commits into from
Mar 18, 2025
Merged

[core] Fix 'filterable' info in field schema #11418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d405a6a
Revert "Revert "[core] Add 'filterable' info in field (#11408)" (#114…
tdraier Mar 17, 2025
22d2d0d
Use optional to fix deserialization
tdraier Mar 17, 2025
6507d71
missing files
tdraier Mar 17, 2025
5ace1f5
use non_filterable
tdraier Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/src/databases/remote_databases/bigquery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl TryFrom<&gcp_bigquery_client::model::table_schema::TableSchema> for TableSc
| FieldType::Interval => TableSchemaFieldType::Text,
},
possible_values: None,
non_filterable: None,
})
.collect(),
)),
Expand Down
5 changes: 5 additions & 0 deletions core/src/databases/remote_databases/salesforce/salesforce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,10 @@ impl SalesforceRemoteDatabase {
QueryDatabaseError::GenericError(anyhow!("Field `soapType` not found"))
})?;

let filterable = field["filterable"].as_bool().ok_or_else(|| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels weird to have the payload different from what we save in DB.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, seen IRL.

QueryDatabaseError::GenericError(anyhow!("Field `filterable` not found"))
})?;

let value_type = match soap_type.to_lowercase().as_str() {
"tns:id" => match field["relationshipName"].as_str() {
Some(rel_name) => TableSchemaFieldType::Reference(rel_name.to_string()),
Expand Down Expand Up @@ -449,6 +453,7 @@ impl SalesforceRemoteDatabase {
name,
value_type,
possible_values: possible_values,
non_filterable: Some(!filterable),
})
})
.collect::<Result<Vec<_>>>()?;
Expand Down
1 change: 1 addition & 0 deletions core/src/databases/remote_databases/snowflake.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl TryFrom<SnowflakeSchemaColumn> for TableSchemaColumn {
name: col.name,
value_type: col_type,
possible_values: None,
non_filterable: None,
})
}
}
Expand Down
24 changes: 23 additions & 1 deletion core/src/databases/table_schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,17 @@ pub struct TableSchemaColumn {
pub name: String,
pub value_type: TableSchemaFieldType,
pub possible_values: Option<Vec<String>>,
// Set to true if this field can't be used in a where clause. False by default
pub non_filterable: Option<bool>,
}

impl TableSchemaColumn {
pub fn new(name: &str, field_type: TableSchemaFieldType) -> Self {
pub fn new(name: &str, field_type: TableSchemaFieldType, non_filterable: Option<bool>) -> Self {
Self {
name: name.to_string(),
value_type: field_type,
possible_values: None,
non_filterable,
}
}

Expand All @@ -88,6 +91,11 @@ impl TableSchemaColumn {
dbml.push_str("']");
}
}

if self.non_filterable.unwrap_or(false) {
dbml.push_str(" [note: non filterable - this field cannot be used in a where clause]");
}

dbml
}
}
Expand Down Expand Up @@ -219,6 +227,7 @@ impl TableSchema {
name: k.clone(),
value_type,
possible_values: Some(vec![]),
non_filterable: None,
};
Self::accumulate_value(&mut column, v);
schema_map.insert(k.clone(), column);
Expand Down Expand Up @@ -472,26 +481,31 @@ mod tests {
name: "field1".to_string(),
value_type: TableSchemaFieldType::Int,
possible_values: Some(vec!["1".to_string(), "2".to_string()]),
non_filterable: None,
},
TableSchemaColumn {
name: "field2".to_string(),
value_type: TableSchemaFieldType::Float,
possible_values: Some(vec!["1.2".to_string(), "2.4".to_string()]),
non_filterable: None,
},
TableSchemaColumn {
name: "field3".to_string(),
value_type: TableSchemaFieldType::Text,
possible_values: None,
non_filterable: None,
},
TableSchemaColumn {
name: "field4".to_string(),
value_type: TableSchemaFieldType::Bool,
possible_values: Some(vec!["TRUE".to_string(), "FALSE".to_string()]),
non_filterable: None,
},
TableSchemaColumn {
name: "field5".to_string(),
value_type: TableSchemaFieldType::Text,
possible_values: Some(vec!["\"not null anymore\"".to_string()]),
non_filterable: None,
},
]);

Expand Down Expand Up @@ -673,21 +687,25 @@ mod tests {
name: "field1".to_string(),
value_type: TableSchemaFieldType::Int,
possible_values: None,
non_filterable: None,
},
TableSchemaColumn {
name: "field2".to_string(),
value_type: TableSchemaFieldType::Float,
possible_values: None,
non_filterable: None,
},
TableSchemaColumn {
name: "field3".to_string(),
value_type: TableSchemaFieldType::Text,
possible_values: None,
non_filterable: None,
},
TableSchemaColumn {
name: "field4".to_string(),
value_type: TableSchemaFieldType::Bool,
possible_values: None,
non_filterable: None,
},
])
}
Expand Down Expand Up @@ -738,10 +756,12 @@ mod tests {
Some(TableSchemaColumn::new(
"no_possible_values",
TableSchemaFieldType::Text,
None,
)),
Some(TableSchemaColumn::new(
"no_possible_values",
TableSchemaFieldType::Text,
None,
)),
TableSchemaFieldType::Text,
None,
Expand Down Expand Up @@ -873,6 +893,7 @@ mod tests {
"2000-01-01 00:00:00".to_string(),
"2000-01-02 00:00:00".to_string(),
]),
non_filterable: None,
}]);

assert_eq!(schema, expected_schema);
Expand Down Expand Up @@ -925,6 +946,7 @@ mod tests {
name: name.to_string(),
value_type,
possible_values: Some(possible_values),
non_filterable: None,
}
}
}
2 changes: 1 addition & 1 deletion front/lib/registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export const BaseDustProdActionRegistry = {
app: {
appId: "b4f205e453",
appHash:
"ae042340a4c705b3cfcc3760a2ffde04c0b3fd246da8ca2e01dd01e2daebcce2",
"d6c762512b6d716f7e13151cf2c245bf51e1923f54859303ca7b144d4ac6dc3f",
},
config: {
MODEL: {
Expand Down
Loading