Message-ID: From: "vlsi (@vlsi)" To: "pgjdbc/pgjdbc" Date: Mon, 23 Mar 2026 17:05:53 +0000 Subject: Re: [pgjdbc/pgjdbc] PR #2646: feat: improve libpq compatibility for URL parsing, environment variables, passfile, and property naming In-Reply-To: References: List-Id: X-GitHub-Author-Login: vlsi X-GitHub-Comment-Id: 4112221148 X-GitHub-Comment-Type: issue_comment X-GitHub-Edited-At: 2026-03-23T17:06:20Z X-GitHub-Issue: 2646 X-GitHub-Repo: pgjdbc/pgjdbc X-GitHub-Type: comment X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/2646#issuecomment-4112221148 Content-Type: text/plain; charset=utf-8 @MarekUniq , the change addresses true issues, so it should be merged sooner or later. Many thanks for pushing this forward. There's challenge with maintaining backward compatibility though. We do not want breaking the apps by merging a PR that adjusts property names. --- Findings so far: 1) It looks like `PgProperty.PG_DBNAME`, `PgProperty.PG_PORT`, `PgProperty.PG_HOST` are wildly used across GitHub repositories, so I am leaning towards un-deprecating them. In other words, we should probably keep them as is, discourage their use in javadoc, and avoid adding `@deprecated` so users don't have to adjust `PgProperty.PG_PORT` -> `PgProperty.PORT` all over the place 2) We need to test somehow that newly added `PgProperty.PORT`-like properties work exactly the same as `PgProperty.PG_PORT`. The existing tests refer to `PgProperty.PG_PORT`, and it is fine to test backward compatibility. However, we should ensure the new ones work 3) `acceptsURL()` can now reject a syntactically valid URL because of external service config. This might be the case like "it worked previously, and now it starts failing" 4) Unknown keys in the Properties argument now cause URL parsing to fail. Previously they were silently ignored, and now they produce failures 5) We should double-check that `System.getProperty("user.name")` still works as a default username. Previously it was working like that, and it looks like `System.getProperty("user.name")` was removed from `Driver.java` for some reason 6) `PGProperty.removeNonExistingProperties` should likely be renamed: it does not remove properties but creates a new `Properties` instance 7) `PGPASSFILE.getDefaultValue()` changed from `"pgpass"` to `null`, and the method is now `@Deprecated`. Code calling `PGEnvironment.PGPASSFILE.getDefaultValue()` will get `null` instead of `"pgpass"`. 8) `Driver.parseURL` was static, and the PR seems to change the names of the properties in the results. External code doing `props.getProperty("PGHOST")` after `Driver.parseURL()` will break. 9) `PgPassParser` and `PgServiceConfParser` made package-private. 10) Service file not found = fatal error. Previously, if no .pg_service.conf file was found, PgServiceConfParser returned null and the driver continued silently. Now it throws JdbcUrlResolverFatalException("Resource file [.pg_service.conf] not found"). This means ?service=foo fails hard if the file doesn't exist, which is correct behavior (matches libpq), but is a change from the previous lenient behavior. --- Frankly, my current understanding is that "fail on error" (==fail on unsupported property, or fail on missing `.pg_service.conf`) should be opt-in, and users should explicitly opt-in to that behavior change