Let the Node test harness use either PGJDBC or pgjdbc-ng
#467
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The two drivers use slightly different URL syntax, and different names for some properties (database.name vs. PGDBNAME, and application.name vs. ApplicationName matter here). Expose a new static s_urlForm and constants URL_FORM_PGJDBC and URL_FORM_PGJDBCNG (and URL_FORM_NONE if no recognized driver has been found) so a client can know what's been selected. To simply count the otherwise-unwanted rows of a ResultSet, pgjdbc-ng allows rs.last();rs.getRow(); but PGJDBC does not, at least under its default for ResultSet scrollability. The method voidResultSetDims gets a change here to not attempt to count the rows when called by peek(); instead, it just returns -1 for rows in that case, which will look odd but play well with both drivers. The PostgreSQL backend sends an interesting message in the case of function/procedure creation with types mentioned that are shells. The message has severity NOTICE but SQLState 42809 (ERRCODE_WRONG_OBJECT_TYPE). For some reason, pgjdbc-ng has not been delivering those, but PGJDBC does, and the heuristic of looking at the class digits to decide if it's ok or not would make the wrong call seeing class 42. Happily, PGJDBC exposes (by casting to PGJDBC-specific types) the severity tag from the backend. So that has to be done here, and done reflectively, to avoid a hard PGJDBC dependency. PGJDBC exposes the SEVERITY (S) tag, but not the SEVERITY_NONLOCALIZED (V) tag that was added in PG 10 (postgres/postgres@26fa446). The upshot is that the rule used here to recognize WARNING will fail if the backend is using a language where it's some other word. A new static method set_WARNING_localized can be used to set the correct tag (PERINGATAN in Indonesian, for example) so the classification will happen correctly for the language selected in the backend. For utility statements with no result, where in pgjdbc-ng there really is no result, in PGJDBC there is a zero row count, the same as for a DML statement that touched no rows. It'll take another commit to make the test state machines work either way.
It would be nice to have a little library of common stateMachine states, heretofore difficult because the next state must be indicated by returning an absolute state number. So, use one of the otherwise-unused parameters of the InvocationHandler functional interface to supply a state with its own state number, so it can make relative moves. To start the library, here is NOTHING_OR_PGJDBC_ZERO_COUNT, because the PGJDBC and pgjdbc-ng drivers differ in handling a utility statement with no result (CREATE EXTENSION, that sort of thing). In pgjdbc-ng, there is no result, while in PGJDBC, there is a zero row count, as would be seen for a DML statement that had not updated anything. So, NOTHING_OR_PGJDBC_ZERO_COUNT simply moves to the numerically next state, after consuming (if the driver is pgjdbc-ng) nothing, or (if the driver is PGJDBC) a zero row count.
This is all about convenience when using JShell interactively, where try-with-resources gets in the way. The VM shutdown hooks to stop and clean up the Node work well, but if any Connection is open at the time, the type of shutdown signalled to the server may not cause it to be closed, leading to a hang. The Java Process API does not expose enough types of OS signals to easily request a faster shutdown. (When the node is using pg_ctl, that's an option, but not when signaling the server directly.) So, have each Node remember the Connections it makes, in a WeakHashMap. They'll be removed from the map once unreachable (and if that happens before they're closed, both drivers include a cleaner task that will eventually close them; there may be a bit of delay, but that's better than the former hang). When stopping a Node, close whatever Connections are still around in the map.
Update the docs chiefly to reflect that either PGJDBC or pgjdbc-ng may be used as the JDBC driver, and how to accommodate the slight differences between the two. Numerous other updates, including to more consistently observe the conventional javadoc style where lead sentences are third-person rather than second. In the course of better documenting when tweaks are applied, caught one place where forWindowsCRuntime would not be applied on Windows. (The known arguments being supplied right there in the code could squeak by without it, but who knows what could be added in a tweak?)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
PostgresNode-like test harness Easter egg included in the package jar has until now only supported pgjdbc-ng (and had some notes in the documentation about issues with PGJDBC that may have been observed at one time, but that I cannot reproduce or vouch for now). So, make the necessary changes inNodeso that either driver can be used, and fix the documentation accordingly.