Message-ID: <gh-pgjdbc-pgjdbc-3631-c2876255597@github.com>
From: "vlsi (@vlsi)" <noreply+vlsi@github.com>
To: "pgjdbc/pgjdbc" <noreply+pgjdbc-pgjdbc@github.com>
Date: Tue, 13 May 2025 12:10:58 +0000
Subject: Re: [pgjdbc/pgjdbc] PR #3631: fix: isValid incorrectly called execute, instead of executeWithFlags fixes Issue #3630
In-Reply-To: <gh-pgjdbc-pgjdbc-3631@github.com>
References: <gh-pgjdbc-pgjdbc-3631@github.com>
List-Id: <gh-pgjdbc-pgjdbc.github.com>
X-GitHub-Author-Login: vlsi
X-GitHub-Comment-Id: 2876255597
X-GitHub-Comment-Type: issue_comment
X-GitHub-Edited-At: 2025-05-13T12:11:48Z
X-GitHub-Issue: 3631
X-GitHub-Repo: pgjdbc/pgjdbc
X-GitHub-Type: comment
X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/3631#issuecomment-2876255597
Content-Type: text/plain; charset=utf-8

How about adding a check to `execute(String sql, int autoGeneratedKeys)` so it rejects unknown flags at least during pgjdbc test execution?

Here's the current code:

```java
    /**
     * ...
     * @param autoGeneratedKeys a flag indicating whether auto-generated keys
     *        should be made available for retrieval;
     *         one of the following constants:
     *         {@code Statement.RETURN_GENERATED_KEYS}
     *         {@code Statement.NO_GENERATED_KEYS}
     */
  @Override
  public boolean execute(String sql, int autoGeneratedKeys) throws SQLException {
    if (autoGeneratedKeys == Statement.NO_GENERATED_KEYS) {
      return execute(sql);
    }
    return execute(sql, (String[]) null);
  }
```

WDYT of adding a check there so it ensures `autoGeneratedKeys` must be either `RETURN_GENERATED_KEYS` or `NO_GENERATED_KEYS`?
Of course it is not backward-compatible, so it should be under `pgjdbc.asserts.statements=true` flag so we could enable it for our tests and the users could enable it to detect wrong API usage in their systems.

----

The test could be located in error-prone (see https://github.com/google/error-prone/issues/3684), or in `javac` itself. I guess it could be a valid `javac` warning if the code casts the object, and then calls the method which was avaliable without a cast.