Default Port Number for SQL Server Shows 1433 but apparently isn't

Version 0.31.2

BACKGROUND

I was attempting to add an SQL Server database and began receiving timeout errors when attempting to save. I debugged by putting the jar file on a server which I knew had access to the database in question. After several abortive attempts, none of which worked, I happened to try something out of desperation.

The Port Number shows a default of 1433.
I hand entered 1433 into the Port Number field.

Lo and behold, the system connected instantly. I can only assume that the internally used default port number for SQL server does not match what is displayed, although I cannot begin to guess why.

Just referencing in case someone stumbles over this:

I’m not sure this is a bug at all. On the same screen, there is a default database name, but it’s obviously not going to be correct. You have to enter a value, that’s all.

It would probably be better if a sentence was used, like with Name and Username.
I can see how having “localhost” and “1433” looks kinda prefilled (use the default).
That’s how a lot of other programs work.

It wouldn’t be except that MySQL shows 3306 as placeholder text and connects without filling in the field. It is a defect because the expected behavior would either be:

  • Make the port a required field and eliminate the distracting number
  • Make the port field behave as MySQL’s connector does; where the placeholder is the default.

Sorry I didn’t give the contrasting information concerning MySQL.

If anyone can point me to where in the code I could look at this, I’ll fix it.

We had to terminate the instance and rebuild. Of course, I forgot about the “default isn’t the default” and wasted over two hours plus another hour of support trying to debug security rule issues with Amazon Web Services. It was only after we established that there were no problems that my brain tickled and I typed 1433 overtop of the default 1433 and the database connected.

@jelrick
I’m pretty sure this is the settings used in the database setup:
https://github.com/metabase/metabase/blob/master/modules/drivers/sqlserver/resources/metabase-plugin.yaml

Wonderful! The bug is obvious. If we compare SQL Server

connection-properties:
   - host
   - merge:
       - port
       - placeholder: 1433

To Oracle

connection-properties:
    - host
    - merge:
        - port
        - default: 1521

It appears the only change necessary may be to convert placeholder to default.

But if you define default, wouldn’t that mean that you actually couldn’t set the field to an empty value?
I think that is required when using that silly Dynamic Port function in SQL Server.

Anyways, it probably shouldn’t say 1433, since you think it defaults to that. Either keep it empty or write Usually 1433 or something else.

I didn’t think the JDBC supported dynamic ports, just the ODBC that Metabase doesn’t use.
I’ve had to change dynamic ports to fixed for SQL with Metabase.

@AndrewMBaines In that case, then it makes even less sense.

@jelrick Can you create a bug and make a write-up of everything gathered here, so the developers don’t have to read through this thread? And then link to the bug here.

1 Like

Yes I can. I’ve put it on my schedule and will do it by later today.

After looking up dynamic ports - never heard of them - my suspicion is that they are a hold over from the early days, before anyone cared about network security. They are a nightmare for anyone trying to configure a firewall.

Dynamic ports “might” make sense for SQL Server Express running locally. Otherwise, they are a security “no-no”; instead of opening one hole in your firewall you have to open a huge range to accommodate one application.

If you have multiple instances of SQL Server running on the same Windows server, dynamic ports are helpful. You have another SQL service running (can’t remember what it’s called) and that tells the client which port to connect to for which instance.
They’re great for internal deployments. Not much use beyond that.
They only normally crop up if you have a non-default instance. Annoyingly, the only time non-default is the default is for SQL Express.

@flamber

@jelrick @AndrewMBaines
It does seem like this was changed to accommodate Dynamic Ports:
https://github.com/metabase/metabase/issues/7597
The change should be to rename the placeholder - not to default it.

I had forgotten that there was already a bug filed on this, so no need for creating a new.
Just go and click the :+1: on the first post:
https://github.com/metabase/metabase/issues/9226

Another interesting issue, which might be caused by the placeholder, but is also related to using the domain\user:
https://github.com/metabase/metabase/issues/6056

Given the debate, my suggestion would be to change the interface slightly for SQL Server:

[ ] Use dynamic ports
[ ] Static port: 1433

Where the 1433 is a default value but only if Static Port is checked.

This change would make the intent explicit rather than relying on arcane knowledge.

1 Like

I’ve been doing some more reading on JDBC and dynamic ports. Looks like they are supported: https://docs.microsoft.com/en-us/sql/connect/jdbc/building-the-connection-url?view=sql-server-2017
Though you’d obviously have to be careful with firewall settings.
However:
For optimal connection performance, you should set the portNumber when you connect to a named instance. This will avoid a round trip to the server to determine the port number. If both a portNumber and instanceName are used, the portNumber will take precedence and the instanceName will be ignored.

1 Like

I think the major point is that, by my estimate, this placeholder has cost my company on the order of $400 in lost productivity already; additionally, we tied up an Amazon tech support guy for over an hour trying to trace down a connectivity issue which turned out to be a misleading bit of text in a field. On top of that was the stress of trying to diagnose a mystery on a mission critical platform in the middle of a major project with a narrow window.

It needs to go; period. If Metabase needs to keep it nullable, change the placeholder text to:

“Enter the port number; e.g. 1433, or leave blank for dynamic ports”

But it needs to get done. I’ll be more than happy to do it and ask for a pull release.

2 Likes