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

MS SQL Server: Support access to DB objects in connection's non default DB #376

Merged
merged 1 commit into from
Jun 6, 2015

Conversation

MMatten
Copy link
Contributor

@MMatten MMatten commented Jun 4, 2015

Add support for access to objects that are not in the current database for the connection/session using the full 3 part name.

Users currently have to rely upon either the default database for the connection (specified for each DB user, or overridden for the connection using either the database= or databaseName= connection parameter), OR, set the default database using a USE <db name> SQL statement.

String bracketedName = enquoteAndJoin(objname.split("\\."), ".", "[", "]");

try (Statement s = currentConnection.createStatement()) {
s.execute("USE " + objectDatabaseName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think this has side effect: the current database is changed; and if something dies - it will remain in the new database.

What about just modifying the query above from sys.columns to something like objectDatabaseName.sys.columns?

@MMatten
Copy link
Contributor Author

MMatten commented Jun 4, 2015

Ah. I've just noticed that I've not checked in the other bit of code that reverts the DB back to the value that I capture in currentDatabaseName! I was executing another statement to revert the current DB right after querying sys.columns so there was no chance of leaving a modified default DB in place for the remainder of the session.

However, your suggestion seems to work perfectly and is much more elegant (and I'm embarrassed I didn't think of it in the first place! 😳).

I'll run the tests over it.

@MMatten
Copy link
Contributor Author

MMatten commented Jun 4, 2015

Hmm. This does involve changing both getAllProcedureParameters and getAllColumns with the similar code but still pretty compact.

@MMatten
Copy link
Contributor Author

MMatten commented Jun 4, 2015

Also tidied up the DB setup script whitespace (we've no style guide for the SQL code - I generally use Steven Feuerstein-like formatting).

String[] objnameParts = procName.split("\\.");
if (objnameParts.length == 3) {
objectDatabaseName = objnameParts[0] + ".";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

That snippet is basically the same as the other. Maybe we can move it to a single place?

@MMatten MMatten force-pushed the access-sqlserver-objects-in-non-default-db branch from c30f775 to caead22 Compare June 5, 2015 09:12
@MMatten
Copy link
Contributor Author

MMatten commented Jun 6, 2015

Do you think it would be better to distribute the tests in the new page I've created into other, existing, test pages or to have an explicit test page for this?

@javornikolov
Copy link
Contributor

A separate test page seems OK to me.

@@ -133,6 +135,16 @@ protected String parseCommandText(String commandText) {
// private static string[] GuidTypes = new string[] { "UNIQUEIDENTIFIER" };
// private static string[] VariantTypes = new string[] { "SQL_VARIANT" };

private String objectDatabasePrefix(String dbObjectName) {

Copy link
Contributor

Choose a reason for hiding this comment

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

In the existing code seems we don't have such blank lines after the method header. And right now to me it looks a bit better without this extra line.

@MMatten
Copy link
Contributor Author

MMatten commented Jun 6, 2015

Ok. Extra blank lines fixed.

@javornikolov
Copy link
Contributor

Now it looks good to me. Might be good to just to clean up the commit history and we can merge then.

@MMatten MMatten force-pushed the access-sqlserver-objects-in-non-default-db branch from 1658852 to 522f02b Compare June 6, 2015 16:09
@MMatten MMatten force-pushed the access-sqlserver-objects-in-non-default-db branch from 522f02b to 917da59 Compare June 6, 2015 16:19
@MMatten
Copy link
Contributor Author

MMatten commented Jun 6, 2015

I've squashed to a single commit and rebased.

javornikolov added a commit that referenced this pull request Jun 6, 2015
…fault-db

Support access to DB objects in connection's non default DB
@javornikolov javornikolov merged commit 75130b1 into master Jun 6, 2015
@javornikolov javornikolov deleted the access-sqlserver-objects-in-non-default-db branch June 6, 2015 18:32
@javornikolov javornikolov changed the title Support access to DB objects in connection's non default DB MS SQL Server: Support access to DB objects in connection's non default DB Aug 15, 2015
@javornikolov javornikolov added this to the 3.2.0 milestone Aug 15, 2015
@javornikolov javornikolov mentioned this pull request Aug 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants