-
Notifications
You must be signed in to change notification settings - Fork 15
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
EthanHardyMongo
wants to merge
25
commits into
mongodb:on-prem-eap
Choose a base branch
from
EthanHardyMongo:SQL-2288
base: on-prem-eap
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+498
−196
Open
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
91e709e
cargo.lock
EthanHardyMongo f9039e0
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 2997ffd
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 71e5ec3
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 6519307
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 914ca8f
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 59a37de
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 74b8d02
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo e7ce323
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 401713b
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 65ff048
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 5bc7a32
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo f5541e3
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 9125f5c
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 8050d4f
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 4e45044
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo 52be409
SQL-2374: Implement columns metadata logic for Direct cluster (#245)
EthanHardyMongo 558358d
Merge remote-tracking branch 'upstream/master'
EthanHardyMongo e7e8234
create mongosqltranslate tests and fix logic
EthanHardyMongo ac45b5d
fix evergreen names
EthanHardyMongo 2ca83d0
remove test flag
EthanHardyMongo d3645cf
rename evergreen mongosqltranslate task
EthanHardyMongo 2f9c330
change version back to 0.0.0
EthanHardyMongo faba35f
change mongosqltranslate library used in evergreen
EthanHardyMongo b17ce9e
respond to some feedback
EthanHardyMongo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ use crate::{ | |
conn::MongoConnection, | ||
err::{Error, Result}, | ||
stmt::MongoStatement, | ||
util::to_name_regex, | ||
util::{databases_filter, to_name_regex}, | ||
BsonTypeInfo, TypeMode, | ||
}; | ||
use constants::SQL_SCHEMAS_COLLECTION; | ||
|
@@ -486,13 +486,7 @@ 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") | ||
&& !db_name.eq("config") | ||
&& !db_name.eq("local") | ||
&& !db_name.eq("system") | ||
}) | ||
.filter(|&db_name| databases_filter(db_name)) | ||
.map(|s| s.to_string()) | ||
.collect() | ||
}, | ||
|
@@ -567,13 +561,17 @@ impl MongoFields { | |
Error::CollectionDeserialization(collection_name.clone(), e) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
let error_checked_sql_get_schema_response = | ||
match sql_get_schema_response { | ||
Ok(sql_get_schema_response) => sql_get_schema_response, | ||
Err(error) => { | ||
// 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() | ||
error_checked_sql_get_schema_response.into() | ||
} else if mongo_connection.cluster_type == MongoClusterType::Enterprise { | ||
let schema_collection = | ||
db.collection::<Document>(SQL_SCHEMAS_COLLECTION); | ||
|
@@ -599,13 +597,14 @@ impl MongoFields { | |
}, | ||
); | ||
|
||
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; | ||
match result_set_schema { | ||
Ok(result_set_schema) => result_set_schema, | ||
Err(error) => { | ||
// 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() | ||
} else { | ||
unreachable!() | ||
}; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this wasn't failing earlier, but I fixed it and added a comment explaining what's going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh actually I think it's because the error it would cause gets pushed into a
warnings
vector instead of causing a crash. This is probably why my tests weren't failing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait never mind. The part of the code that would fail was never called by my tests. That's why this wasn't causing any failures.