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.

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

SQL-2288: Create mongosqltranslate integration tests #251

Open
wants to merge 25 commits into
base: on-prem-eap
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
91e709e
cargo.lock
EthanHardyMongo Mar 6, 2024
f9039e0
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Mar 6, 2024
2997ffd
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Mar 12, 2024
71e5ec3
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Mar 12, 2024
6519307
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Mar 26, 2024
914ca8f
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Apr 3, 2024
59a37de
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Apr 15, 2024
74b8d02
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Apr 19, 2024
e7ce323
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Jun 3, 2024
401713b
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Jun 6, 2024
65ff048
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Jun 27, 2024
5bc7a32
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Jul 31, 2024
f5541e3
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Aug 22, 2024
9125f5c
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Sep 19, 2024
8050d4f
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Sep 26, 2024
4e45044
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Oct 4, 2024
52be409
SQL-2374: Implement columns metadata logic for Direct cluster (#245)
EthanHardyMongo Oct 7, 2024
558358d
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo Oct 7, 2024
e7e8234
create mongosqltranslate tests and fix logic
EthanHardyMongo Oct 14, 2024
ac45b5d
fix evergreen names
EthanHardyMongo Oct 15, 2024
2ca83d0
remove test flag
EthanHardyMongo Oct 15, 2024
d3645cf
rename evergreen mongosqltranslate task
EthanHardyMongo Oct 15, 2024
2f9c330
change version back to 0.0.0
EthanHardyMongo Oct 15, 2024
faba35f
change mongosqltranslate library used in evergreen
EthanHardyMongo Oct 15, 2024
b17ce9e
respond to some feedback
EthanHardyMongo Oct 17, 2024
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
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion constants/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "constants"
version = "0.0.0"
version = "2.0.0"
authors = [
EthanHardyMongo marked this conversation as resolved.
Show resolved Hide resolved
"Ryan Chipman <[email protected]>",
"Natacha Bagnard <[email protected]>",
Expand Down
1 change: 1 addition & 0 deletions constants/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub const DRIVER_NAME: &str = "MongoDB Atlas SQL ODBC Driver";
pub const DBMS_NAME: &str = "MongoDB Atlas";
pub const ODBC_VERSION: &str = "03.80";
pub const DRIVER_SHORT_NAME: &str = "mongodb-odbc";
pub const SQL_SCHEMAS_COLLECTION: &str = "__sql_schemas";

lazy_static! {
pub static ref DRIVER_METRICS_VERSION: String = format!(
Expand Down
15 changes: 13 additions & 2 deletions core/src/col_metadata.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::collections::HashMap;

use crate::{
json_schema::{
simplified::{Atomic, ObjectSchema, Schema},
BsonTypeName,
},
BsonTypeInfo, Error, Result, TypeMode,
};
use bson::{Bson, Document};
use definitions::{Nullability, SqlCode, SqlDataType};
use itertools::Itertools;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

// Metadata information for a column of the result set.
// The information is to be used when reporting columns information from
Expand Down Expand Up @@ -173,10 +173,21 @@ pub struct VersionedJsonSchema {
// Struct representing the ResultSetSchema.
#[derive(Serialize, Deserialize, PartialEq, Debug, Clone, Default)]
pub struct ResultSetSchema {
#[serde(rename = "result_set_schema")]
pub schema: crate::json_schema::Schema,
#[serde(skip_serializing_if = "Option::is_none")]
pub select_order: Option<Vec<Vec<String>>>,
}

impl ResultSetSchema {
pub fn from_sql_schemas_document(doc: &Document) -> bson::de::Result<Self> {
let as_bson = Bson::Document(doc.clone());
let deserializer = bson::Deserializer::new(as_bson);
let deserializer = serde_stacker::Deserializer::new(deserializer);
Deserialize::deserialize(deserializer)
}
}

impl From<SqlGetSchemaResponse> for ResultSetSchema {
fn from(sql_get_schema_response: SqlGetSchemaResponse) -> Self {
Self {
Expand Down
8 changes: 7 additions & 1 deletion core/src/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,13 @@ impl MongoCollections {
.unwrap()
.iter()
// MHOUSE-7119 - admin database and empty strings are showing in list_database_names
.filter(|&db_name| !db_name.is_empty() && !db_name.eq("admin"))
.filter(|&db_name| {
!db_name.is_empty()
&& !db_name.eq("admin")
&& !db_name.eq("config")
&& !db_name.eq("local")
&& !db_name.eq("system")
})
.filter(|&db_name| is_match(db_name, db_name_filter, accept_search_patterns))
.map(|val| async move {
CollectionsForDb {
Expand Down
8 changes: 4 additions & 4 deletions core/src/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::mongosqltranslate::{
use crate::odbc_uri::UserOptions;
use crate::{err::Result, Error};
use crate::{MongoQuery, TypeMode};
use constants::DRIVER_ODBC_VERSION;
use constants::DRIVER_METRICS_VERSION;
use lazy_static::lazy_static;
use mongodb::{
bson::{doc, Bson, UuidRepresentation},
Expand Down Expand Up @@ -154,7 +154,7 @@ impl MongoConnection {
}

fn is_libmongosqltranslate_compatible_with_driver_version() -> Result<bool> {
let command = CheckDriverVersion::new(DRIVER_ODBC_VERSION.clone());
let command = CheckDriverVersion::new(DRIVER_METRICS_VERSION.clone());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The DRIVER_ODBC_VERSION wasn't a valid semver version, so I switched to the DRIVER_METRICS_VERSION.


let command_response = libmongosqltranslate_run_command(command)?;

Expand Down Expand Up @@ -196,7 +196,7 @@ impl MongoConnection {

let uuid_repr = user_options.uuid_representation;

load_mongosqltranslate_library();
load_mongosqltranslate_library(false);

let (is_libmongosqltranslate_compatible_with_driver_version, libmongosqltranslate_version) =
if get_mongosqltranslate_library().is_some() {
Expand Down Expand Up @@ -243,7 +243,7 @@ impl MongoConnection {
.is_some_and(|is_compatible| is_compatible)
{
return Err(Error::LibmongosqltranslateLibraryIsIncompatible(
&DRIVER_ODBC_VERSION,
&DRIVER_METRICS_VERSION,
libmongosqltranslate_version
.ok_or(Error::EmptyLibmongosqltranslateVersion)?,
));
Expand Down
8 changes: 7 additions & 1 deletion core/src/databases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,13 @@ impl MongoDatabases {
})
.unwrap()
.iter()
.filter(|&db_name| !db_name.is_empty() && !db_name.eq("admin"))
.filter(|&db_name| {
Copy link
Member

Choose a reason for hiding this comment

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

Since this list of databases and filter is in a couple places, can we add a helper function to filter out the databases we don't want?

!db_name.is_empty()
&& !db_name.eq("admin")
&& !db_name.eq("config")
&& !db_name.eq("local")
&& !db_name.eq("system")
})
.map(|s| s.to_string())
.collect();

Expand Down
2 changes: 1 addition & 1 deletion core/src/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub enum Error {
"The ODBC driver version `{0}` is incompatible with libmongosqltranslate version `{1}`"
)]
LibmongosqltranslateLibraryIsIncompatible(&'static str, String),
#[error("The following collections were not found in the `__sql_schemas` collection: {0:?}")]
#[error("The following collection(s) were not found in the `__sql_schemas` collection: {0:?}")]
SchemaDocumentNotFoundInSchemaCollection(Vec<String>),
#[error(
"The libmongosqltranslate command `{0}` failed. Error message: `{1}`. Error is internal: {2}"
Expand Down
85 changes: 68 additions & 17 deletions core/src/fields.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
cluster_type::MongoClusterType,
col_metadata::{MongoColMetadata, ResultSetSchema, SqlGetSchemaResponse},
collections::MongoODBCCollectionSpecification,
conn::MongoConnection,
Expand All @@ -7,9 +8,10 @@ use crate::{
util::to_name_regex,
BsonTypeInfo, TypeMode,
};
use constants::SQL_SCHEMAS_COLLECTION;
use definitions::{Nullability, SqlDataType};
use mongodb::{
bson::{doc, Bson},
bson::{doc, Bson, Document},
results::CollectionType,
};
use once_cell::sync::OnceCell;
Expand Down Expand Up @@ -484,7 +486,13 @@ impl MongoFields {
.unwrap()
// MHOUSE-7119 - admin database and empty strings are showing in list_database_names
.iter()
.filter(|&db_name| !db_name.is_empty() && !db_name.eq("admin"))
.filter(|&db_name| {
!db_name.is_empty()
&& !db_name.eq("admin")
&& !db_name.eq("config")
&& !db_name.eq("local")
&& !db_name.eq("system")
})
.map(|s| s.to_string())
.collect()
},
Expand Down Expand Up @@ -542,23 +550,66 @@ impl MongoFields {
// The collection does not match the filter, moving to the next one
continue;
}
let get_schema_cmd = doc! {"sqlGetSchema": collection_name.clone()};

let db = mongo_connection.client.database(&self.current_db_name);
let current_col_metadata_response: Result<SqlGetSchemaResponse> =
mongodb::bson::from_document(
db.run_command(get_schema_cmd).await.unwrap(),
)
.map_err(|e| {
Error::CollectionDeserialization(collection_name.clone(), e)
});
if let Err(error) = current_col_metadata_response {
// If there is an Error while deserializing the schema, we won't show any columns for it
warnings.push(error);
continue;
}
let current_col_metadata_response: ResultSetSchema =
current_col_metadata_response.unwrap().into();

let current_col_metadata_response: ResultSetSchema = if mongo_connection
Comment on lines +549 to +550
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code was merged into master with SQL-2374

.cluster_type
== MongoClusterType::AtlasDataFederation
{
let get_schema_cmd = doc! {"sqlGetSchema": &collection_name};

let sql_get_schema_response: Result<SqlGetSchemaResponse> =
mongodb::bson::from_document(
db.run_command(get_schema_cmd).await.unwrap(),
)
.map_err(|e| {
Error::CollectionDeserialization(collection_name.clone(), e)
Copy link
Member

Choose a reason for hiding this comment

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

How about an integration test that confirms we get this on a deserialization error?

});

if let Err(error) = sql_get_schema_response {
// If there is an Error while deserializing the schema, we won't show any columns for it
warnings.push(error);
continue;
}

sql_get_schema_response.unwrap().into()
} else if mongo_connection.cluster_type == MongoClusterType::Enterprise {
let schema_collection =
db.collection::<Document>(SQL_SCHEMAS_COLLECTION);

let schema_doc: Document = mongo_connection
.runtime
.block_on(async {
schema_collection
.find_one(doc! {
"_id": &collection_name
})
.await
.map_err(Error::QueryExecutionFailed)
})?
.ok_or(Error::SchemaDocumentNotFoundInSchemaCollection(vec![
collection_name.clone(),
]))?;

let result_set_schema: Result<ResultSetSchema> =
ResultSetSchema::from_sql_schemas_document(&schema_doc).map_err(
|e| {
Error::CollectionDeserialization(collection_name.clone(), e)
},
);

if let Err(error) = result_set_schema {
// If there is an Error while deserializing the schema, we won't show any columns for it
warnings.push(error);
continue;
}

result_set_schema.unwrap()
Copy link
Member

Choose a reason for hiding this comment

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

How about something like this to avoid the unwrap, it has similar behavior:

match result_set_schema {
    Ok(schema) => schema,
    Err(error) => {
        warnings.push(error);
        continue;
    }
}

} else {
unreachable!()
};

match current_col_metadata_response.process_collection_metadata(
&self.current_db_name,
collection_name.as_str(),
Expand Down
29 changes: 22 additions & 7 deletions core/src/mongosqltranslate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ fn get_library_name(library_type: &str) -> String {
}
}

fn get_library_path(library_type: &str) -> PathBuf {
let lib_name = get_library_name(library_type);
fn get_library_path() -> PathBuf {
let lib_name = get_library_name(LIBRARY_NAME);
let mut path = PathBuf::from(LIBRARY_INSTALL_PATH);
path.push(lib_name);
path
Expand All @@ -63,12 +63,12 @@ fn get_mock_library_path() -> PathBuf {
// and is responsible for determining the library name and path.
// The library name and path are determined based on the operating system and architecture.
// It is stored in a static variable to ensure that it is only loaded once.
pub fn load_mongosqltranslate_library() {
pub fn load_mongosqltranslate_library(mock_library: bool) {
INIT.call_once(|| {
let library_path = if cfg!(test) {
let library_path = if mock_library {
get_mock_library_path()
} else {
get_library_path(LIBRARY_NAME)
get_library_path()
Comment on lines +66 to +71
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The mock_library is now only used by the unit test in this file; all the other tests can use the real library now, so I figured we could do this instead to make things easier.

};

match unsafe { Library::new(library_path.clone()) } {
Expand Down Expand Up @@ -221,6 +221,7 @@ impl CheckDriverVersion {
}

#[derive(Debug, Serialize, Deserialize)]
#[serde(tag = "command_type")]
pub enum CommandResponse {
Comment on lines +224 to 225
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The deserialization wasn't working without this.

Translate(TranslateCommandResponse),
GetNamespaces(GetNamespacesCommandResponse),
Expand Down Expand Up @@ -308,7 +309,7 @@ pub(crate) fn libmongosqltranslate_run_command<T: CommandName + Serialize>(

let decomposed_returned_doc = unsafe { run_command_function(libmongosqltranslate_command) };

let command_response_doc: Document = unsafe {
let mut command_response_doc: Document = unsafe {
bson::from_slice(
Vec::from_raw_parts(
decomposed_returned_doc.data.cast_mut(),
Expand All @@ -320,6 +321,20 @@ pub(crate) fn libmongosqltranslate_run_command<T: CommandName + Serialize>(
.map_err(Error::LibmongosqltranslateDeserialization)?
};

let command_type = if command_response_doc.get_str("error").is_ok() {
"Error"
} else {
match T::command_name() {
"getNamespaces" => "GetNamespaces",
"translate" => "Translate",
"getMongosqlTranslateVersion" => "GetMongosqlTranslateVersion",
"checkDriverVersion" => "CheckDriverVersion",
_ => unreachable!(),
}
};

command_response_doc.insert("command_type", command_type);

let command_response = CommandResponse::from_document(&command_response_doc)?;

if let CommandResponse::Error(error_response) = command_response {
Expand All @@ -340,7 +355,7 @@ mod unit {

#[test]
fn library_load_and_run_command_test() {
load_mongosqltranslate_library();
load_mongosqltranslate_library(true);
assert!(get_mongosqltranslate_library().is_some());

let run_command = get_run_command_fn_ptr().expect("Failed to load runCommand symbol");
Expand Down
Loading