Support JDBC validate connection

[xsgao-github]

Description

• Support HEAD /v1/statement, this allows client to validate authentication without submitting a query.
• Add validateConnection property that allows validating connection when making a JDBC connection - SQLException does not return the correct SQLState when making connection failed #16504
• Fix JDBC Connection.isValid(int) - fix JDBC Connection.isValid(int) should verify access token expiration #22684

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## Section
* Fix JDBC `Connection.isValid(int)` which does not validate connection and/or credentials. ({issue}`22684`)
* Add `validateConnection` JDBC property that allows validating connection when making a JDBC connection. ({issue}`16504`)

[wendigo]

I'd prefer to not to add this to the StatementClientV1 itself - it's not needed there and the changes to remove the query from the constructor are wrong.

Instead, I'd just add it to JDBC only and use the fact that, TrinoConnection has the authenticated client already. With it, it's just a matter of:

        OkHttpClient.Builder builder = new OkHttpClient.Builder(client) // if you want some custom timeouts
                .connectTimeout(10, TimeUnit.SECONDS)
                .readTimeout(10, TimeUnit.SECONDS);

        OkHttpClient timeoutClient = builder.build();
        timeoutClient.
                newCall(new Request.Builder().head()
                        .url(trinoUri.getUri() + "/v1/statement")
                        .build())
                .execute()
                .isSuccessful();

You can add this alone to isValid(int timeout) method. The only thing there is to handle 404 gracefully.

[xsgao-github]

You can add this alone to isValid(int timeout) method.

But this undercut the pattern that JDBC delegates function calls to trino-client.

Given the fact that /v1/statement serves functions as two end-points, POST with a query body and a bunch of Trino headers, and HEAD without body or Trino headers, and you all disagree on making query optional, I am leaning to create a new AuthenticationClient object which connects to /v1/statement as well.

[aalbu]

The reason to control this behavior through a property was indeed the extra round-trip. The argument was that since creating connections in Trino is basically free currently, there may be client applications that create connections for every query and in a scenario like that, adding an extra round-trip would introduce a performance regression. EXECUTE IMMEDIATE was introduced to address similar concerns.

[wendigo]

@aalbu If the goal is to implement isValid than we can remove validation when connection is created (as the credentials will be validated when query is submitted) and leave it only for isValid. Since isValid is an explicit call, we will avoid this extra round trip and there will be no need for extra property.

[aalbu]

The goal is to make Trino play nicer with BI tools. Most JDBC drivers ensure that connections are valid at creation time and also let you do a proper validity check). BI tools tend to misbehave when the behavior is different (i.e. you can create connections with wrong credentials or calling isValid() returns true when credentials are invalid).

[wendigo]

Do we know whether isValid is enough or not?

[xsgao-github]

Do we know whether isValid is enough or not?

Fixing isValid will help use case when connection pool is in place. But here we want to fix three things:

1. Validate connection at getConnection.
2. Return correct SQLState as getConnection.
3. Validate connection at isValid.

[wendigo]

I've applied some structural changes and cleanups and squashed commits together.

[xsgao-github]

I've applied some structural changes and cleanups and squashed commits together.

@wendigo is co-author now, @aalbu @electrum & @martint would you please review and approve this PR?

[wendigo]

@mosabua PTAL at jdbc.md change

[wendigo]

@xsgao-github why have you added a merge commit here? We are not doing that

[mosabua]

Minor doc changes and then good to go.

[xsgao-github]

@xsgao-github why have you added a merge commit here? We are not doing that

Yes, I clicked on Update branch button.

[mosabua]

As part of docs fix also please rebase and squash commits .. and do that manually .. as you saw the GitHub UI messes that up with a merge commit ;-)

[wendigo]

@xsgao-github it's not needed

[mosabua]

Ship it @wendigo !

[xsgao-github]

@wendigo Did we backport these to LTS? https://github.com/trinodb/trino/pull/24127/files#diff-38acb3ee3a4ac23e3d6158d73ed3c2689c05b5082c26e12f00a8309387236bf7R158-R165

[mosabua]

@xsgao-github there is no such thing as a Trino LTS release. Please keep questions relevant to Trino and related open source projects.