-
-
Notifications
You must be signed in to change notification settings - Fork 24
Oracle provider #34
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
base: develop
Are you sure you want to change the base?
Oracle provider #34
Conversation
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.
Thanks! Sorry it's taken me so long to have a look. Main thing is the question about commenting out oldVersionParam - the rest of the things I can fix up myself.
oldVersionParam.ParameterName = "OldVersion"; | ||
oldVersionParam.Value = oldVersion; | ||
command.Parameters.Add(oldVersionParam); | ||
//var oldVersionParam = command.CreateParameter(); |
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.
Is it right that these are now commented out?
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.
What is role of the parameter ? Is it used for any other provider ? If I uncomment it Oracle wll throw error if the parameter is not used in sql statement
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.
It's there if custom database providers want to use it. Removing it would be a breaking change.
public static readonly string MSSQL = @"Server=.;Database=SimpleMigratorTests;Trusted_Connection=True;"; | ||
public static readonly string MySQL = @"Server=localhost;Database=SimpleMigrator;Uid=SimpleMigrator;Pwd=SimpleMigrator;"; | ||
public static readonly string PostgreSQL = @"Server=localhost;Port=5432;Database=SimpleMigrator;User ID=SimpleMigrator;Password=SimpleMigrator"; | ||
public static readonly string Oracle = @"Data Source=localhost:32769/ORCLPDB1.localdomain;User ID=testmig;Password=testmig"; |
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.
Mind changing these to SimpleMigrator/SimpleMigrator?
/// <returns>SQL to fetch the current version from the version table</returns> | ||
protected override string GetCurrentVersionSql() | ||
{ | ||
return $@"select version from (select version from {this.TableName} order by id desc) where rownum=1"; |
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.
Different case to the rest of the queries?
public OracleDatabaseProvider(Func<DbConnection> connectionFactory) | ||
: base(connectionFactory) | ||
{ | ||
TableName = "VERSION_INFO"; |
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.
Is using all caps a particularly strong oracle-ism? I.e. is there a compelling reason to move away from the name used by other database providers?
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.
Yes, it is standard, oracle names are by default case insensitive and even if you will use small caps it will be stored in dictionary as all caps
86b13b4
to
1d70d94
Compare
Oracle Provider using Oracle Managed Driver