Created attachment 143927 [details]
stacktrace

Actually, it doesn't look like any queries work.

Hello, could you please specify driver version and PostgreSQL version?

Thanks

The driver (provided by NetBeans) is labeled as "postgresql-9.2-1002.jdbc4.jar". The supported version of the 9.2 driver according to the postgresql site is -1004. I'm connecting to version 9.3.2.

The site also mentions that the JDBC4 version should only be used for JDK 6. JDKs 7 and 8 should use the JDBC41 version. From that the proper version of the 9.2 driver should therefore be "postgresql-9.2-1004.jdbc41.jar".

I tried adding the the latest version (postgresql-9.3-1100.jdbc41.jar) to NetBeans, but no difference (same error).

I figure the missing spaces in the JDBC URL is just a logging issue, but I thought I'd mention it just in case.

This happens with each driver - and my patch is responsible - sorry!

This is the problem: The fix for bug #121186 contained an unnessary and unintended change. Concrete: 

http://hg.netbeans.org/core-main/rev/1ff219d7fa20

3.33/3.34 and 3.40/3.41

Libor, could you please revert that part of the change?

The issue is not Postgres specific, and there appears to be two bugs here. It's caused by specifying a different display name in the "New Connection" wizard, but that is *not* the source of the issue (see below). Before explaining I need to verify what I believe to be the intended behavior of NetBeans:

1. When a DB connection is first created, the display name is set to the connection
   URL. Since showing the URL again in the tool tip would be redundant, whatever
   property that the tool tip uses (toString?) is set to "Database Connection".

2. If the DB connection's display name is later renamed to something other than the
   URL, the tool tip's property is then set to the URL, since seeing the URL in the
   tool tip is more useful than just seeing "Database Connection".

The two bugs:

1. If the display name is *not* the URL but the tool tip's property (toString?)
   *is*, NetBeans gives this issue. I imagine this is because the code that
   connects to the DB does not correctly identify the existing connection as the
   connection it's trying to establish. Netbeans then attempts to create a new
   connection which throws the AssertionError, since the code that creates a
   connection correctly sees that the connection is already present.

   Interestingly, if neither the display name nor the tool tip's property is the
   URL, there is no problem. I imagine this is due to an incorrect attempt to fix
   this issue in the past.

2. Even though it seems to work, the "rename" context menu item is *not* properly
   changing the display name, which causes the tool tip's property to not be set
   to the URL. I imagine this is also due to an incorrect attempt to fix this issue
   in the past.

These are likely *not* the actual bug, even though they appear to cause the issue:

1. Changing the connection name in the wizard. The wizard is properly setting the
   display name, which triggers the change in the tool tip's property.

2. Changing the display name directly from the "Properties" dialog, which also is
   properly changing the display name.

By changing the display name from the "Properties" dialog, I can replicate the issue with Derby (and I assume any DB) as well.

I think all of those changes are correct, save for getName/setName in ConnectionNode. Of course, I'm just looking at the code now. Please see comment #8.

Matthias, I don't think it's necessary to revert your patch. I tried to fix the bug: http://hg.netbeans.org/core-main/rev/8669b285c85b What do you think about it? You patch works and this issue does not occur any more. I think it did not make sense to call findConnectionNode with getDisplayName() instead of getName() if findConnectionNode uses getName to compare...

Please verify the fix.

(In reply to Libor Fischmeistr from comment #10)
> Matthias, I don't think it's necessary to revert your patch. I tried to fix
> the bug: http://hg.netbeans.org/core-main/rev/8669b285c85b What do you think
> about it? You patch works and this issue does not occur any more. I think it
> did not make sense to call findConnectionNode with getDisplayName() instead
> of getName() if findConnectionNode uses getName to compare...
> 
> Please verify the fix.

Libor, sorry as though it looks ok, it is not. Change the display name and then invoke "rename" action via F2 or rename menu entry. In that case the "physical name" of the connection is exposed and not the display name used, that from my POI, would be the "right thing".

I think athompson and you got me wrong - I meant that _part_ of the patch should be reverted - to be more concrete (the following diffs are :

Index: db/src/org/netbeans/modules/db/explorer/node/ConnectionNode.java
--- db/src/org/netbeans/modules/db/explorer/node/ConnectionNode.java
+++ db/src/org/netbeans/modules/db/explorer/node/ConnectionNode.java
@@ -409,13 +409,13 @@
     
     @Override
     public String getName() {
-        return connection.getName();
+        return connection.getDisplayName();
     }
 
     @Override
     public void setName(String name) {
         String old = getName();
-        connection.setName(name);
+        connection.setDisplayName(name);
         fireNameChange(old, name);
     }


This reestablishes the old state - without sacrificing the other changes. While looking at this some more - the following should be corrected aswell:

1. I saw an error in an error message
2. findConnectionNode is inconstently called -- based on my assumption I search by displayName and use that consistently
3. the missing documentation for findConnectionNode is problematic - now the assumption is documented and while the next coder might not like the decision, at least it's known that it is intentional

Index: db/src/org/netbeans/modules/db/explorer/DatabaseConnection.java
--- db/src/org/netbeans/modules/db/explorer/DatabaseConnection.java
+++ db/src/org/netbeans/modules/db/explorer/DatabaseConnection.java
@@ -1185,7 +1185,7 @@
     }
     
     public void refreshInExplorer() throws DatabaseException {
-        final ConnectionNode connectionNode = findConnectionNode(getName());
+        final ConnectionNode connectionNode = findConnectionNode(getDisplayName());
         if (connectionNode != null) {
             RP.post(
                 new Runnable() {
@@ -1216,7 +1216,7 @@
     public void showConnectionDialog() {
         try {
             final ConnectionNode cni = findConnectionNode(getDisplayName());
-            assert cni != null : "DatabaseConnection node found for " + this;
+            assert cni != null : "DatabaseConnection not node found for " + this;
             if (cni != null && cni.getDatabaseConnection().getConnector().isDisconnected()) {
                 Mutex.EVENT.readAccess(new Runnable() {
                     @Override
@@ -1253,7 +1253,17 @@
         }
     }
 
-    // Needed by unit tests as well as internally
+    /**
+     * Find a connection node using the supplied name.
+     * 
+     * <p>Assumption: the name of the connection node is the display name of the connection</p>
+     * 
+     * <p>Needed by unit tests as well as internally</p>
+     * 
+     * @param connection display name of the connection for which the connection node should be found
+     * @return
+     * @throws DatabaseException 
+     */
     public static ConnectionNode findConnectionNode(String connection) throws DatabaseException {
         assert connection != null;

I forgot to mention: I tested with main golden. I applied the change from 8669b285c85b manually. My changes are also relative to that state (so without your change, which would have to be reverted to reach my tested state).

Mathias, I know I've said this before, but thanks for all your hard work. If you're at a NetBeans conference and the NetBeans guys don't buy you a beer, I'll personally fly there and buy you one.  :)

That being said, I think the new assert has the "not" in the wrong place.

...and thanks to you Libor and all the NetBeans guys for making a great product.

*** Bug 239139 has been marked as a duplicate of this bug. ***

I hope this is the last change :-) http://hg.netbeans.org/core-main/rev/6f8e74a6e569

I've reverted my changeset and applied Matthias's patch.

Now it should work properly.

Please verify this changeset.

Thank you

(In reply to Libor Fischmeistr from comment #16)
> I hope this is the last change :-)
> http://hg.netbeans.org/core-main/rev/6f8e74a6e569
> 
> I've reverted my changeset and applied Matthias's patch.
>
> Please verify this changeset.
> 

Verified - I checked the changeset, cloned core-main (jay 1,5h cloning ...) and build that. I checked:

- Different rename variants
- Restarting netbeans in between and so invoking the "select connection"-dialog
- Code completion
- Invoked hibernate reverse engineering dialog/wizard

This should cover the duplicates and reported problems.

Thank you for you patience!

(In reply to athompson from comment #13)
> That being said, I think the new assert has the "not" in the wrong place.

You are right and Libor corrected that - thanks again!

*** Bug 240560 has been marked as a duplicate of this bug. ***

Integrated into 'main-silver', will be available in build *201401170001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/8669b285c85b
User: Libor Fischmeistr <lfischmeistr@netbeans.org>
Log: #240434: [Regression] AssertionError: DatabaseConnection node found for <drivername>

Yep. One nitpick is that before if you renamed a connection, the tool tip text would be set to the JDBC URL so that information wouldn't be lost. Any chance of getting that back?

(In reply to athompson from comment #20)
> Yep. One nitpick is that before if you renamed a connection, the tool tip
> text would be set to the JDBC URL so that information wouldn't be lost. Any
> chance of getting that back?

Could you please recheck this? From my testing (I just checked my 7.4 installation @work) the situation in the upcoming 8.0 is the same as 7.4 with regard to renames (that is: the display name is always used).

My change accidentally exposed the database connection name, but not the JDBC URL. The JDBC URL is part of the name by construction.

If you still think this should be changes (for example that the JDBC URL should always be shown in the connection node tooltip), please file another bug/RFE. At least I would appretiate it, if this bug could rest in peace.