This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
See Issue 122944 for details
Andrei suggests try { conn.getMetaData().getTables(null, null, " ", new String[] { "TABLE" }).close(); } catch (SQLException e) { return false; } return true; but my concern is that this could return a *LOT* of tables for a large schema. I suggest adding a filter to the table names so that we either get one or no results if it is successful, e.g. try { conn.getMetaData().getTables(null, null, "MRBADEA", new String[] { "TABLE" }).close(); } catch (SQLException e) { return false; } return true; That way Andrei will also be immortal :)
Another problem with getTables() - there are a number of drivers that don't support this. Now we would break these drivers not only for the explorer but for the SQL Editor and other uses as well. I wonder if there is another approach. Let me think about it a bit.
Well, after further thought, I believe this is probably the best choice, with the caveat that you may get some false negatives for drivers that don't support DatabaseMetaData.getTables(). Any attempt to issue "generic SQL" will yield more false negatives -- I don't think there is *any* SQL that works across all vendors :)
This is a defect rather than a feature because without it we are generating spurious unhelpful exceptions like "Insufficient data while reading from the network - expected a minimum of 6 bytes and received only -1 bytes" (see Issue 143798)
What is wrong with sending " " as the table name pattern? It should only return a table named " ", at least according to the Javadoc.
This API change allows a user of the API to validate that a connection is working before issuing commands to the database. This allows code using this API to display a useful message to the user rather than display exceptions that are ugly and hard to interpret. Note the two issues that depend on this already.
Created attachment 67571 [details] Patch file for this API change
Andrei sends an email saying: === David, I don't think it is enough to display an error message. For Derby at least, when the server has been stopped it can just be started. Actually, even if the server can't be started, or the network is down, the clients of the test() method should not be forced to invent their own message. The API should do that for them. So the test() method should put the connection in the disconnected state, so that the next call to showConnectionDialog() tries to reconnect and it fails with a standard error message. Moreover, how the clients are going to use the test() method? They will get a connection through getJDBCConnection(), then use test(), and eventually call showConnectionDialog() if needed. Looks like a pattern that will end up copy-pasted throughout the code. Perhaps the clients would actually want a Connection getJDBCConnection(boolean test); method, not a test() method. When called with a false parameter, it will do the same as getJDBCConnection(). When called with true, it will either return null if the connection is not connected, or test the connection and return null or the java.sql.Connection. When the test fails, the connection is also put in the disconnect state. #### and I respond: I like these ideas - keep it simple for the user API and don't require copy/paste of boilerplate code. I also like the bit of marking the connection disconnected so that the next call to showConnectionDialog() does the right thing. Also the connect() method needs to do the right thing. I'll do another revision.
I'm adding getJDBCConnection(test). But one question I have about your comment, Andrei <Andrei> Actually, even if the server can't be started, or the network is down, the clients of the test() method should not be forced to invent their own message. The API should do that for them. </Andrei> What were you thinking here? The getJDBCConnection() method shouldn't post a dialog if the connection is invalid, as it's not part of the UI layer.
I meant that the client shouldn't be forced to do: Connection conn = dbconn.getJDBCConnection(); if (!dbconn.test()) { // Display ad-hoc "connection broken" message, expecting that the connection // can't be reconnected -- which is true in most cases. } Instead, they should do: Connection conn = dbconn.getJDBConnection(true); // Null is returned here if the connection is broken. if (conn == null) { ConnectionManager.getDefault().showConnectionDialog(dbconn); } This way, if the connection is broken beyond repair and it can't be reconnected, the sCD() will display the standard error message.
Created attachment 67871 [details] Updated patch for this API change
Andrei, et. al. -- I have attached an updated patch which adds the getJDBCConnection(test) method, which marks the connection as closed if there is a problem. I modified the original getJDBCConnection() to call getJDBCConnection(false). I am not sure about this. I would feel it would be more robust to have it call getJDBCConnection(true) but I am concerned that this is an incompatible change. Do others have opinions about this. I am on the fence.
The API looks fine, but I have a couple of notes on the documentation: [AB01] "This change allows you to make sure a DatabaseConnection is valid". You do not test the DatabaseConnection, but the underlying physical JDBC connection. The DatabaseConnection is always valid. The same applies to the test-connection use case in the arch.xml. [AB02] I would suggest dropping "before you use it to issue commands to the database". The user can do whatever he wants with the connection, so best to keep the description generic. [AB03] "it marks the DatabaseConnection as invalid". It disconnects the connection, not marks it as invalid. [AB04] Typo in "Get a JDBCConnection" -- no such thing as a JDBCConnection. Actually, the Javadoc of the new getJBDCConnection(boolean) method should be based on and include all the details in getJDBCConnection(), while also documenting the new behavior and parameter. [AB05] Please state the in Javadoc that the method that getJDBCConnection(true) uses to test the connection is unspecified, but it can be a long-duration operation such as executing a statement against the database. Also mention that it should not be called in the event dispatching thread. Also, the patch seems to include unintentional modifications to DatabaseConnectionTest.java. You are right that getJDBCConnection() should call getJDBCConnection(false) for backward compatibility reasons, and also for performance reasons, as stated in [AB05].
I applied Andrei's comments on the docs and have committed the change. Now the issues that depend on this feature can be fixed. fa862747712d
Integrated into 'main-golden', available in build *200808280201* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/fa862747712d User: David Van Couvering <davidvc@netbeans.org> Log: #143837: Need test() method to DatabaseConnection
Integrated into 'main-golden', available in build *200808300201* on http://bits.netbeans.org/dev/nightly/ Changeset: http://hg.netbeans.org/main/rev/6ac44f704347 User: David Van Couvering <davidvc@netbeans.org> Log: #143837: Update the version for the spec
Verified.