pgjdbc/pgjdbc GitHub issues and pull requests (mirror)  
help / color / mirror / Atom feed
[pgjdbc/pgjdbc] issue #3928: Copilot scan showed two minor code issues in ConnectionFactoryImpl
2+ messages / 2 participants
[nested] [flat]

* [pgjdbc/pgjdbc] issue #3928: Copilot scan showed two minor code issues in ConnectionFactoryImpl
@ 2026-02-05 03:50 "timgstewart (@timgstewart)" <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: timgstewart (@timgstewart) @ 2026-02-05 03:50 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

Please read https://stackoverflow.com/help/minimal-reproducible-example 

**Describe the issue**

This isn't an issue I see manifested in anyway, and it does not affect me operationally.  I have a copy of the code in my codebase and our copilot scanner flagged a couple very minor issues.  I wanted to notify you here rather than just moving along and ignoring it.

Due to our many postgresql instances that we need to connect to and detect the timezone (an issue described at https://github.com/pgjdbc/pgjdbc/issues/2927) we have a copy of ConnectionFactoryImpl from (https://github.com/pgjdbc/pgjdbc/blob/REL42.7.9/pgjdbc/src/main/java/org/postgresql/core/v3/Connecti...) which adds conditional at line 415 (v42.7.9):

    if (info==null || !Boolean.parseBoolean(info.getOrDefault("server_timezone", "false").toString())) {
      paramList.add(new StartupParam("TimeZone", createPostgresTimeZone()));
     }

Anyway, I recently updated our driver version, and updated our copy of this class.  It passed through our co-pilot which showed the following minor bugs which I think are legit:

**Bug in conditional logic**: Line 228 parses protocolVersion as an integer and assigns it to protocolMinor, but line 229 immediately overwrites protocolMinor with 0, losing the parsed value. Line 229 should assign 0 to protocolMajor instead (since when there's no decimal point, the entire value is the minor version and major should default to 0 or 3).

Line 226 looks like:
226:        int decimal = protocolVersion.indexOf('.');
227:        if (decimal == -1) {
228:          protocolMinor = Integer.parseInt(protocolVersion);
229:          protocolMinor = 0;

You can see that the set value on 228 is discarded.

Second issue it called out was:

**Bug in conditional logic**: The ternary operator is inverted. When i > 0 (not the first iteration), it should prepend a comma, but the current code adds an empty string. When i == 0 (first iteration), it adds a comma which is incorrect. The condition should be: i > 0 ? "," : "" (swap the true and false branches).

This is line 764:

764:                errorMessage  += i > 0 ? "" : "," + pgStream.receiveString();
suggestion:    errorMessage  += (i > 0 ? "," : "") + pgStream.receiveString();

You can see that the , separation on 764 inserts a comma when the iteration index is 0.  So you get:

,val1val2val3val4

**Driver Version?** 42.7.9

**Java Version?** 25

**OS Version?** N/A

**PostgreSQL Version?** 17.7

**To Reproduce**
Steps to reproduce the behaviour:

No steps to reproduce, just alerting you to a minor code scanner defect that popped up.

**Expected behaviour**
A clear and concise description of what you expected to happen.
And what actually happens

**Logs**
If possible PostgreSQL logs surrounding the occurrence of the issue
Additionally logs from the driver can be obtained adding

Using the following template code make sure the bug can be replicated in the driver alone.
```
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.Properties;

public class TestNullsFirst {
    public static void main(String []args) throws Exception {


        String url = "jdbc:postgresql://localhost:5432/test";

        Properties props = new Properties();
        props.setProperty("user", "test");
        props.setProperty("password", "test");
        try ( Connection conn = DriverManager.getConnection(url, props) ){
            try ( Statement statement = conn.createStatement() ) {
                try (ResultSet rs = statement.executeQuery( "select lastname from users order by lastname asc nulls first") ){
                    if (rs.next())
                        System.out.println( "Get String: " + rs.getString(1));
                }
            }
        }
    }
}
```


^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: [pgjdbc/pgjdbc] issue #3928: Copilot scan showed two minor code issues in ConnectionFactoryImpl
@ 2026-02-05 11:21 "davecramer (@davecramer)" <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: davecramer (@davecramer) @ 2026-02-05 11:21 UTC (permalink / raw)
  To: pgjdbc/pgjdbc <[email protected]>

does this fix the issue https://github.com/pgjdbc/pgjdbc/pull/3929

^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2026-02-05 11:21 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-05 03:50 [pgjdbc/pgjdbc] issue #3928: Copilot scan showed two minor code issues in ConnectionFactoryImpl "timgstewart (@timgstewart)" <[email protected]>
2026-02-05 11:21 Re: [pgjdbc/pgjdbc] issue #3928: Copilot scan showed two minor code issues in ConnectionFactoryImpl "davecramer (@davecramer)" <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox