-
Notifications
You must be signed in to change notification settings - Fork 4
This PR upgrades PL/Java to version 1.6.10 and adapts it for Cloudberry Database, along with adding a Docker-based CI testing environment. #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Now that 1.5.8 is released, the next release should include an extension SQL file allowing upgrade from 1.5.8.
This picks up the PostgreSQL 14 (#359) changes, as well as the #340 (pg_do_encoding_conversion) and #355 (autovacuum) fixes, and some doc mentions of JEP 411. Revised the ErrorData.c patch slightly to avoid a C compiler warning for an unused variable (easier to spot in 1.6 where it's not in a flood of other warnings). That change can be backpatched to 1.5 for consistency.
PL/Java has never, until now, implemented anything by reaching into unexposed JDK innards. The need to do so now is fairly irksome. Java, any language, is classic infrastructure. Other layers, like this, are built atop it, and others in turn use those layers. The idea that the language developers would arrogate to themselves the act of sending an inappropriately low-level message directly to ultimate users, insisting that the stack layers above cannot intercept it and notify the higher-level users in terms that fit the abstractions meaningful there, leaves an uneasy picture of how a development team can begin to lose sight of who is providing what to whom and why.
Adapt beginEnforcing() to throw exceptions with decent explanations on Java > 17 if UnsupportedOperationException has been caught (suggest adding -Djava.security.manager=allow, for whatever good it might do), or if getSecurityManager() returns the wrong thing (suggest trying -Dorg.postgresql.pljava.policy.enforcement=none to ignore the fact, though again this may do little good or lead simply to a different later failure). Add doc comments to various InstallHelper methods lacking them.
Because PL/Java 1.6 builds with --release 9, no @SuppressWarnings annotations need to be added for JEP 411, as the deprecation markings introduced in the JEP are not visible as of release 9. I will turn on the maven-compiler-plugin's <showDeprecation> anyway, as its default of false just seems wrong to me. I'll create a branch from this point that has the @SuppressWarnings annotations that would otherwise need to be added, so they will be on record, but there is no need to merge them in for PL/Java 1.6. May as well not clutter the code with them unnecessarily.
Having suppressed the boilerplate warning from Java itself (which was going to spam standard error for every backend that starts PL/Java, ending up in the server log if logging_collector is set), set about supplying a more usefully worded warning at more strategically chosen times, that will mention the PL/Java wiki/JEP-411 page for details on the road map ahead. The idea is to produce no more than one warning per backend, and only at times when something PL/Java-administrative in nature is happening, to maximize the chance that the person who occasions the warning is a person it will mean something to. So, deliver the warning: 1. when PL/Java itself is installed or upgraded (really, any time its library is loaded by LOAD, directly or by the extension script, even if it's the version already installed and nothing else happens). 2. when functions are declared or redeclared that use it (as detected by the validator handler). 3. when pg_upgrade is run on a database that has PL/Java installed. 1 and 2 are only delivered if the Java major version is 12 or later. The idea is that if a site tends to stick with long-term-support Java releases, there will be plenty of time on 17 LTS for warnings, and no need to deliver them while running 11 LTS. At another site that follows along at a faster pace with non-LTS releases, start with the warnings as soon as running anything later than 11. 3 is unconditional because under pg_upgrade a JVM doesn't get launched to determine its version. 1 and 2 also defer the warning to the commit of a top-level transaction that contained the triggering event(s). If the transaction doesn't commit, the warning is squelched to avoid adding noise to other messages possibly indicating something went wrong.
They should not be counted as ng test results.
Factor the check for a MappedUDT into a checkTypeMappedUDT function, for symmetry with Function_checkTypeBaseUDT. Should make it simpler to grasp what's going on with the two distinct flavors of UDT. Do not expose global functions for obtaining handles for the two support functions that only BaseUDTs have, which are only obtained within Function.c anyway.
Includes a bit of refactoring so internal_call_handler doesn't separately call Function_getFunction and Function_invoke.
This is done from the C side and, perhaps more controversially, using JNI to poke a non-exposed field in java.lang.Thread. PL/Java had not engaged in such tomfoolery before the JEP 411 warning situation required it, and this is now the second instance. It bears mentioning that, from ongoing discussion of JEP 411, the Java developers seem to be taking the position that any project wishing to offer some kind of permission enforcement post-JEP 411 is going to be more or less expected to do so by poking at non-exposed innards using JNI, JVMTI, or the like. In some post-1.6 PL/Java version, this kind of thing will have to become more common. In this case, it is simply to finally start setting the context loader appropriately (I do think it's arguably a correctness issue, as discussed in #361) while adding as little overhead as possible to the hot path of invoking functions. This code will detect, and fall back to not managing the context loader, if the target JRE lacks the expected field. It can also be ineffective if an application has subclassed Thread with a different behavior for getContextClassLoader. It seems tolerable to leave such cases unhandled. Because in PL/Java 1.6 the division of labor still has parameter and return value conversions done from the C side before and after the target method is invoked, and those conversions could involve Java UDTs whose support methods are not bound to context loaders in their own right, it makes sense to have the C code impose the target method's context loader in advance of the parameter conversions, with restoration only after return value conversion. Any UDT methods then consistently see the target method's context loader, whether they are being invoked 'outside' it for parameter or return conversion, or 'inside' it during PreparedStatement / ResultSet / SQLInput / SQLOutput operations. Likewise, leave class initialization to happen, as it normally would, on first use of a function from the class, when the right context loader will be in place. It had been pulled up to validation time to address a small quirk where OpenJ9's class resolution is so lazy it can overlook missing-dependency problems HotSpot would flag, but it's more fuss than the small quirk is worth. This commit introduces some C code in the typedef-function-signature pattern, rather than the typedef-function-pointer pattern prevalent in PostgreSQL upstream, as discussed in [0]. Being valid C89/"ANSI", and with PostgreSQL's documentation stipulating C89 or ANSI compilers as early as PG 8.4 [1], this is expected to cause no problems in an environment that can build any PG version PL/Java 1.6 supports. [0] https://www.postgresql.org/message-id/615C847A.7070609%40anastigmatix.net [1] https://www.postgresql.org/docs/8.4/source-format.html
Slightly increases overhead for the java_thread_pg_entry == allow case. Bottom lines now (X5650, relative to 7a7f394): java_thread_pg_entry == allow: 1.6 µs per call, every call requires setting 0.7 µs per call, consecutive calls use same loader java_thread_pg_entry != allow: 0.7 µs per call, every call requires setting 0.2 µs per call, consecutive calls use same loader
Done as a system property interpreted in InstallHelper, rather than as a GUC, in (a) recognition that it's obscure and may be little used, and (b) anticipation that other allowed settings may evolve in ways that might be easier to parse/validate in Java.
Arguably, these changes should have been made as soon as 1.6.x targeted Java 9 as baseline.
Add a 'builtin' argument (default true) requiring Java to find its own in-the-box XSLT implementation; when false, an implementation via the (now properly managed!) context classloader may be found; for example, XSLT 3.0 transforms can be used if the schema class path includes Saxon. When not using the builtin implementation, treat as warnings, rather than errors, any failure to recognize specific features/attributes the builin implementation is expected to recognize. It's appropriate for the static transformer factory s_tf to be the builtin one.
The earlier validator patch now backed out was motivated not just by the extreme laziness in OpenJ9 (which can happily give you a method handle before even trying to resolve any of the method's symbolic references), but also by getting an unhelpful message even on HotSpot, which is better at detecting such issues, but the exception that really shows what went wrong can be a couple getCause()s down. At least that second issue has an easy solution: just generate a better message.
Because the Java API treats null as a meaningful value for setContextClassLoader, something other than null must be used as the sentinel "didn't set one" value. But choosing some arbitrary int bits other than NULL to use as a sentinel pointer value could be trouble on some architectures. Assuming no logic errors, whatever sentinel value is used here will never get passed to JNI and used as an object reference, but should such an error exist, the results could be undefined unless the value is an actual JNI global reference to a ClassLoader instance. So, to play it safe, just make a dummy one of those to use as the sentinel. If any sequence of mishaps leads Java to use it as a class loader, it will throw null pointer exceptions, but that beats undefined behavior. Invocation_pushBootContext can just use null; its uses may precede the allocation of the sentinel, and popBootContext doesn't try to restore anything anyway.
No functional change.
Check at runtime whether a MappedUDT is being applied to a PostgreSQL type declared with typlen of -2 (a variable-length, NUL-terminated representation), and report an error if so. Explain in the developer documentation why the implementation cannot support the use. It already didn't work, but an explanatory error message is a better way of not working. Addresses #370.
Making this change manually, as dependabot would probably make it in the wrong branch. 1.10 requires Java 8, so no backpatching to REL1_5_STABLE. This whole dependency on ant can probably be removed easily now, by moving the build.xml logic into pom.xml as a scripted-goal.
... in the course of which, noticing that a change being described didn't have enough detail in the JavaDoc, added that too.
Now that 1.6.3 is released, the next release should include an extension SQL file allowing upgrade from 1.6.3.
Merges PR #533.
A not-uncommon mistake by newcomers to PL/Java is to catch an exception raised during a call back into PostgreSQL (such as through the internal JDBC interface), but then to try to proceed without either rolling back to a previously-established Savepoint or (re-)throwing the same or another exception. That leaves the PostgreSQL transaction in an undefined state, and PL/Java will reject subsequent attempts by Java code to call into PostgreSQL again. Those later rejections raise exceptions that may have no discernible connection to the original exception that was mishandled, and may come from completely unexpected places (a ClassNotFoundException from PL/Java's class loader, for example). Meanwhile, the actual exception that was originally mishandled to cause the problem may never be logged or seen, short of connecting with a debugger to catch it when thrown. The result is an overly-challenging troubleshooting process for such a common newcomer mistake. This commit patches PL/Java to retain information about a PostgreSQL error when it is raised and until it is resolved by rolling back to a prior Savepoint or until exit of the PL/Java function. If a subsequent attempt to call into PostgreSQL is rejected because of the earlier error, the exception thrown at that point can supply, with getCause(), the original exception at the root of the problem. If the remembered PostgreSQL error still has not been resolved by a rollback when the function returns (normally or exceptionally) to PostgreSQL, exception stack traces will be logged. The log level depends on whether the function has returned normally or exceptionally and also on whether any later attempts to call into PostgreSQL did get made and rejected. Details are in a new documentation section "Catching PostgreSQL exceptions in Java", which see. Example code is also added.
Also, the first commit neglected to say "Addresses #523", so here's that.
Merges PR #535.
No issues observed in compiling manually (without the Maven directive specifying an earlier --release) on Oracle JDK 25 GA. cd pljava-api/src/main/java /var/tmp/jdk-25/bin/javac -d ../../../target/classes/ \ -Xlint:unchecked -Xlint:-removal --module-version 1.6-SNAPSHOT \ $(find . -name '*.java') cd ../../.. /var/tmp/jdk-25/bin/jar cf target/pljava-api-1.6-SNAPSHOT.jar \ -C target/classes . cd ../pljava/src/main/java /var/tmp/jdk-25/bin/javac --module-version 1.6-SNAPSHOT \ -d ../../../target/classes/ \ -h ../../../target/javah-include \ --module-path ../../../../pljava-api/target/pljava-api-1.6-SNAPSHOT.jar \ --processor-module-path \ ../../../../pljava-api/target/pljava-api-1.6-SNAPSHOT.jar \ -Xlint:unchecked -Xlint:-removal $(find . -name '*.java') cd ../../../ /var/tmp/jdk-25/bin/jar cf target/pljava-1.6-SNAPSHOT.jar \ -C target/classes . cd ../pljava-examples/src/main/java /var/tmp/jdk-25/bin/javac -d ../../../target/classes/ \ --module-path ../../../../pljava-api/target/pljava-api-1.6-SNAPSHOT.jar \ --processor-module-path \ ../../../../pljava-api/target/pljava-api-1.6-SNAPSHOT.jar \ --class-path \ ~/.m2/repository/net/sf/saxon/Saxon-HE/10.9/Saxon-HE-10.9.jar: \ -Xlint:unchecked -Xlint:-removal \ --add-modules org.postgresql.pljava $(find . -name '*.java') cd ../../../target/classes cp -r ../../src/main/resources/* . zip -r ../pljava-examples-1.6-SNAPSHOT.jar * # zip because jar m doesn't preserve order of manifest entries cd ../../../ # with dir of intended pg_config version on PATH: mvn clean install --projects pljava-pgxs,pljava-so,pljava-packaging
Merges PR #542.
mvn versions:set -DgenerateBackupPoms=false -DnewVersion=1.6.10
|
|
|
HI @Mulily0513, please take this as a reference to merge this PR: https://lists.apache.org/thread/w6x45nqpx5gtf6xwn5zxv61lp8ky2ydq |
ok |
81f2add to
d3132a2
Compare
…ry fork. # Conflicts: # pljava-ant/pom.xml # pljava-api/pom.xml # pljava-deploy/pom.xml # pljava-examples/pom.xml # pljava-examples/src/main/java/org/postgresql/pljava/example/AnyTest.java # pljava-examples/src/main/java/org/postgresql/pljava/example/MetaDataInts.java # pljava-examples/src/main/java/org/postgresql/pljava/example/annotation/Parameters.java # pljava-packaging/pom.xml # pljava-so/build.xml # pljava-so/pom.xml # pljava-so/src/main/c/Backend.c # pljava-so/src/main/c/Function.c # pljava-so/src/main/c/InstallHelper.c # pljava-so/src/main/c/PgSavepoint.c # pljava-so/src/main/c/SPI.c # pljava-so/src/main/c/SQLInputFromTuple.c # pljava-so/src/main/c/Session.c # pljava-so/src/main/c/type/AclId.c # pljava-so/src/main/c/type/Array.c # pljava-so/src/main/c/type/HeapTupleHeader.c # pljava-so/src/main/c/type/JavaWrapper.c # pljava-so/src/main/c/type/LargeObject.c # pljava-so/src/main/c/type/Portal.c # pljava-so/src/main/c/type/Time.c # pljava-so/src/main/c/type/Timestamp.c # pljava-so/src/main/c/type/TupleDesc.c # pljava-so/src/main/c/type/TupleTable.c # pljava-so/src/main/c/type/Type.c # pljava-so/src/main/include/pljava/Backend.h # pljava-so/src/main/include/pljava/pljava.h # pljava/pom.xml # pljava/src/main/java/org/postgresql/pljava/elog/ELogHandler.java # pljava/src/main/java/org/postgresql/pljava/internal/Backend.java # pljava/src/main/java/org/postgresql/pljava/sqlj/Loader.java # pom.xml # src/site/markdown/releasenotes.md.vm
- Add GitHub Actions workflow to run PL/Java build/test against Cloudberry - Provide reproducible ubuntu22.04 docker-compose environment - Build/install Cloudberry demo cluster inside container - Build/install PL/Java and run built-in regression (make installcheck) - Update expected output for Cloudberry-specific differences
- add 1.6.10 extension files and install layout - add legacy example shims for regression - GPDB-specific guards and replicated sqlj tables - adjust loader/fallback behavior - add Cloudberry docker CI and remove lazypg workflow
b8abcbf to
ab73fd1
Compare
Key Changes
Change logs
Contributor's checklist
Here are some reminders before you submit your pull request: