Mutable static in SQLServerClobBase

[marschall]

The static field com.microsoft.sqlserver.jdbc.SQLServerClobBase#logger is mutable and set by every constructor invocation. It is only used in com.microsoft.sqlserver.jdbc.SQLServerClobBase#free(). As a result with logger is used there is pretty much random. My recommendation would be to make it static final.

[cheenamalhotra]

Hi @marschall The logger in SQLServerClobBase (abstract class) is present as an object holder for passed logger object while creating a new object of the class that extends it, SQLServerClob or SQLServerNClob.

I agree with your point of finalizing logger objects to stop mutation, but in this case, the intention is to log messages as the extended class, and not as per the abstract class and that's the reason, it's passed by constructor.

I think the current implementation is intentional and need not be changed since this is an exceptional case, and not a regular logger scenario.

[marschall]

My point is not so much about finalizing but about semantics. There is only one logger in for all instances of SQLServerClobBase and subclasses. Whoever created the last instance overwrites the logger of everybody else. It does not matter what you pass as an argument to the super constructor, the next call to the super constructor will overwrite it.

I do not agree with your statement that this is an exceptional case since SQLServerClob and SQLServerNClob always pass a logger. They both pass in a different logger. So it is very well possible that you get a SQLServerNClob logger in a SQLServerClob instance and vice versa.

[cheenamalhotra]

I understand your point now.

The reason behind this behavior is basically because logger is static member in SQLServerClobBase. Removing static keyword will resolve this issue and result in maintaining logger for every object as instantiated. On the other hand, making it static final would result in logging messages with SQLServerClobBase abstract class and not subclasses SQLServerClob or SQLServerNClob.

I am creating a PR which makes logger final in abstract class SQLServerClobBase and not static such that it is instantiated based on subclass name (using getClass().getName()). Therefore, logger is removed from subclasses since it was used for only passing through constructor.

Let me know if the PR resolves your query.

[cheenamalhotra]

Hi @marschall the fix is merged and released in v6.3.5 - please review it and let us know if it works for you. Closing the issue for now.