Conversation
Add RootClass support in DBI->connect: when the caller passes
{ RootClass => 'MyDBI' } or invokes a DBI subclass directly
(MyDBI->connect), the returned outer handles (drh/dbh/sth) are
reblessed into ${RootClass}::dr/::db/::st, matching real DBI's
subclass-dispatch behaviour.
Changes in src/main/perl/lib/DBI.pm:
- connect wrapper detects RootClass from explicit attr or
caller-class; eagerly requires the module (skipping require if
the package is already defined inline).
- Failed RootClass load dies unconditionally so $@ is set for
the eval-and-inspect pattern the tests use.
- Added _apply_root_class / _rebless_outer helpers; they stash
RootClass on the inner dbh so prepared sths inherit it.
Changes in src/main/perl/lib/DBI/_Handles.pm:
- _new_sth reblesses the sth outer into ${RootClass}::st when
the parent dbh has RootClass set.
- _new_sth now inherits the error-handling attributes
(RaiseError, PrintError, PrintWarn, RaiseWarn, HandleError,
HandleSetErr, ShowErrorStatement, Warn) from the parent dbh,
so set_err on a sth correctly fires RaiseError/HandleError.
- DBI::_::OuterHandle::_dispatch_packages detects the dr/db/st
suffix via isa() when the ref isn't exactly DBI::{dr,db,st},
so subclass-reblessed handles still route through AUTOLOAD.
- DBD::_::db::clone propagates RootClass + CompatMode/RaiseError/
PrintError forward into the cloned handle.
Per-test deltas (./jperl t/X.t directly):
- t/30subclass.t: 19/43 → 43/43
- t/06attrs.t: 142/166 → 145/166
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Document the Phase 8 RootClass work that just landed, and lay out Phase 9: replace our hand-rolled DBI.pm + _Handles.pm (~2300 lines) with upstream DBI.pm 1.647 + DBI::PurePerl + a small shim, then reimplement the PurePerl-skipped XS features (Profile, Callbacks, Kids, swap_inner_handle) in Java. Based on a spike that ran upstream DBI 1.647 + DBI::PurePerl under PerlOnJava: 8 test files jumped from partial-fail to full-pass (01basics 130/130, 31methcache 49/49, 12quote, 14utf8, 20meta, 30subclass, 05concathash, 11fetch). Two PerlOnJava interpreter bugs surfaced and are listed as prerequisites: - Qualified method call does not walk target's @isa ($x->Bar::method() with @bar::ISA doesn't find inherited methods) - ClassCastException in { COND ? (%h1, %h2) : %h1 } under the interpreter backend Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
$obj->Pkg::method(...) was doing a direct symbol-table lookup of Pkg::method and failing if the method wasn't defined directly in Pkg, even when it was inherited via @pkg::ISA. Per Perl semantics, qualified method calls should walk Pkg's inheritance chain (not the invocant's), same as a regular method lookup but anchored at Pkg instead of ref($obj). Minimal repro: package Foo; sub hello { ... } package Bar; our @isa = ("Foo"); bless({}, "main")->Bar::hello(); # Real Perl: dispatches Foo::hello # PerlOnJava (before fix): "Undefined subroutine &Bar::hello" Impact: DBI.pm 1.647 calls $drh->DBD::_::dr::STORE($k, $v) and relies on @dbd::_::dr::ISA = qw(DBD::_::common) plus DBD::_::common::STORE. Also affects any CPAN module using this idiom (common in modules with "method wrapper" patterns). Fix: in RuntimeCode.call, split the qualified method on the last ::, then use InheritanceResolver.findMethodInHierarchy starting from the target package. callCached is unaffected because its fast path already bails out on names containing "::" and falls through to call(). Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This is the big architectural switch discussed in the design doc.
Our hand-rolled DBI.pm (841 lines) and DBI/_Handles.pm (1484 lines)
re-implemented most of what upstream DBI + DBI::PurePerl already
provide. This PR replaces them with upstream unchanged.
- src/main/perl/lib/DBI.pm — upstream DBI 1.647 DBI.pm (8696 lines)
with a single 4-line PerlOnJava patch: force
$ENV{DBI_PUREPERL}=2 before the XSLoader-vs-PurePerl decision
block, so DBI::PurePerl is always used and XSLoader::load is
never attempted (PerlOnJava has no XS).
- src/main/perl/lib/DBI/PurePerl.pm — upstream DBI::PurePerl 1.47
unchanged.
- src/main/perl/lib/DBI/_Handles.pm, _Utils.pm — deleted.
Per-test DBI suite deltas (./jperl ~/.cpan/build/DBI-1.647-5/t/X.t):
| Test | Phase 8 (pre) | Phase 9 (this) |
|---|---|---|
| 01basics.t | 100/130 (halts) | **130/130** |
| 03handle.t | 94/137 | 134/137 |
| 06attrs.t | 145/166 | 164/166 |
| 15array.t | 16/55 | **50/55** |
| 30subclass.t | 43/43 | 43/43 |
| 31methcache.t | 24/49 | **49/49** |
| 12quote.t | 5/10 | 10/10 |
| 14utf8.t | 10/16 | 15/16 |
| 20meta.t | 3/8 | 8/8 |
| 09trace.t | 99/99 | 1/99 (PerlOnJava bug) |
| profile/callbacks/kids | partial | SKIP (legit PurePerl skips) |
8 files go from partial-fail to full-pass; 4 more go from badly
broken to ≥95%. Profile/Callbacks/Kids/swap_inner_handle are
legitimately SKIPped by PurePerl — these are the XS-only features
a follow-up commit will reimplement in Java.
Known regressions / TODO for follow-up PRs:
1. **t/09trace.t regresses from 99/99 to 1/99** because of a
separate PerlOnJava interpreter bug: ClassCastException in
`{ COND ? (%h1, %h2) : %h1 }` hash-list construction under
the interpreter fallback path. Needs minimal repro + fix.
2. **JDBC-backed DBDs (DBD::SQLite, DBD::Mem) stop working via
`DBI->connect("dbi:SQLite:…", …)`.** Root cause: upstream
DBI.pm's `sub connect` replaces the Java-registered
`DBI::connect`, so the JDBC path never runs. Follow-up PR
will:
a. Change DBI.java to register methods under
DBD::JDBC::{dr,db,st}::* (a proper upstream-style DBD)
instead of under package DBI.
b. Turn DBD::SQLite and DBD::Mem into proper upstream-style
drivers that inherit from DBD::JDBC and provide
_dsn_to_jdbc translation.
c. The bundled JDBC drivers (SQLite, H2, PostgreSQL, etc.)
still work — the route from user code is just via a
different plumbing.
No bundled unit tests exercise DBI->connect with a JDBC
driver (only examples/dbi.pl), so this doesn't regress
`make`.
3. **PurePerl-skipped XS features** (Profile, Callbacks, Kids
bookkeeping, swap_inner_handle) will be reimplemented in Java
as the project's equivalent of the upstream XS layer. This
addresses the 91 Profile tests that were the biggest block in
the design doc.
This commit is the infrastructure change. The follow-up commits
ride on top.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Follow-up to Phase 9 (upstream DBI.pm + DBI::PurePerl switch), which
disabled the JDBC-backed DBDs because our Java-registered methods
were under package `DBI` and got shadowed when upstream DBI.pm
defined its own `sub connect`, `sub prepare`, etc.
This commit re-plumbs the JDBC path into a proper upstream-style DBD:
**Java (`DBI.java`, `PerlModuleBase.java`, `GlobalContext.java`):**
- New `PerlModuleBase.registerMethodInPackage(targetPackage, perlName,
javaName)` helper that forces registration under an arbitrary Perl
package regardless of the module's moduleName.
- `DBI.initialize()` now registers methods under DBD::JDBC::{dr,db,st}:
- `DBD::JDBC::dr::connect`, `::data_sources`
- `DBD::JDBC::db::prepare`, `::disconnect`, `::last_insert_id`,
`::commit`, `::rollback`, `::begin_work`, `::ping`,
`::table_info`, `::column_info`, `::primary_key_info`,
`::foreign_key_info`, `::type_info`, `::get_info`
- `DBD::JDBC::st::execute`, `::fetchrow_arrayref`,
`::fetchrow_hashref`, `::rows`, `::bind_param`,
`::bind_param_inout`, `::bind_col`
- All `bless ... "DBI::db"` / `"DBI::st"` / `"DBI::dr"` call sites in
DBI.java are retargeted to the DBD::JDBC equivalents so
upstream's dispatch sees the driver-specific ImplementorClass.
- `DBI.initialize()` is now called from `GlobalContext.initializeJVM()`
— with upstream DBI.pm running in DBI_PUREPERL=2 mode, XSLoader is
never called, so we can't rely on the XSLoader path for init.
**Perl (`DBD/JDBC.pm`, `DBD/SQLite.pm`):**
- New `DBD::JDBC` base driver provides the upstream-shape `driver()`
factory, `dr` / `db` / `st` classes that inherit from DBD::_::{...}
and wire the Java methods into them.
- `DBD::JDBC::st` aliases `fetch` and `fetchrow` to
`fetchrow_arrayref` so Perl's MRO stops on DBD::JDBC::st rather
than falling through to DBD::_::st's default-fallback fetchrow_*
methods (which call each other recursively assuming the driver
has implemented at least one).
- `DBD::SQLite` now inherits from `DBD::JDBC`; its `::dr::connect`
overrides the base to translate the DSN suffix to a JDBC URL via
`_dsn_to_jdbc` before delegating into Java.
**Deleted: `src/main/perl/lib/DBD/Mem.pm`.** The bundled upstream
DBI 1.647 ships a real pure-Perl DBD::Mem (installed into
`~/.perlonjava/lib/DBD/Mem.pm` by `jcpan -i DBI`). Our shim that
redirected dbi:Mem: to SQLite's in-memory mode was shadowing it —
removing the shim lets the real upstream DBD::Mem take effect,
which matters for the DBI test suite's `t/54_dbd_mem.t`.
**Verified:**
- `./jperl examples/dbi.pl` runs end-to-end (connect + do + prepare +
execute + fetchrow_arrayref + fetchrow_hashref + disconnect all
via dbi:SQLite:).
- `make` passes (all unit tests).
- DBI test suite pass rates maintained at Phase 9 levels
(01basics 130/130, 06attrs 164/166, 17handle_error 84/84,
30subclass 43/43, 31methcache 49/49, 15array 50/55, etc.).
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Document the architectural switch to upstream DBI.pm + DBI::PurePerl (Phase 9) and the JDBC-path restoration via DBD::JDBC (Phase 9b). Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Reinstates t/09trace.t from 1/99 back to 99/99 after the Phase 9
DBI switch exposed this latent bug.
Symptom: when upstream DBI 1.647's `DBI->connect(...)` runs under
the JVM→interpreter fallback path, the connect closure hits:
my $apply = { ($override_attr) ? (%$attr, %$override_attr) : %$attr };
and at scope exit the interpreter crashes with:
Interpreter error in (eval 31): ClassCastException
RuntimeList cannot be cast to RuntimeScalar
[java:BytecodeInterpreter.java:177]
i.e. the my-scalar register for `$apply` contained a RuntimeList
instead of a RuntimeScalar. The root cause is a compiler bug in
the interpreter backend: under some ternary-in-hashref shapes,
the my-scalar slot receives the ternary's LIST-context result
rather than the hashref scalar. A minimal repro outside DBI's
connect closure has not yet been isolated.
This commit does NOT fix the compiler bug. It adds a defensive
guard in SCOPE_EXIT_CLEANUP that tolerates non-scalar values in
a my-scalar slot — SCOPE_EXIT_CLEANUP only needs to act on real
RuntimeScalars (for IO-owner fd recycling, refCount decrement,
and captureCount tracking); anything else has nothing to clean.
Without the guard, every such case aborts the program; with it,
the program runs correctly (the RuntimeList is simply discarded
at scope exit along with other transient register values).
Verified:
- t/09trace.t: 1/99 → **99/99** (full pass)
- No regression on any other DBI test (01basics 130/130,
06attrs 164/166, 08keeperr 88/91, 15array 50/55, 17handle_error
84/84, 30subclass 43/43, 31methcache 49/49, 03handle 134/137).
- `make` passes (all unit tests).
Follow-up: track down the interpreter-emitter bug that puts a
RuntimeList in a scalar register. Likely in TernaryOperatorNode
visit under LIST context inside a HashLiteralNode, where the
result register's type isn't coerced to scalar before assignment
to a my-scalar slot.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Added commit The root cause is a compiler bug in the interpreter backend: Follow-up issue to fix the emitter root cause tracked in the plan doc — likely in
|
|
Followed up on the root-cause analysis per your concern — the defensive guard IS the correct fix (not just a workaround). Added commit
Commit message has the full analysis. An optional follow-up (not in this PR) would be an emitter-side change: emit unconditional All DBI tests still at the Phase 9b numbers, |
…r slot
Amends the earlier "defensive guard" commit with the proper root-cause
analysis and a regression test. This fix is semantically correct, not
just defensive.
## Root cause
When a `my` scalar is declared inside a short-circuiting expression,
e.g.:
if ( ref($x) and isa($x, 'HASH')
and defined((my $h_new = $x)->{key}) ) { ... }
the compiler allocates a register for `$h_new` at compile time, but
the MY_SCALAR opcode that initialises that register is only emitted
as part of the `defined(...)` sub-expression. If an earlier `and`
operand short-circuits false, the MY_SCALAR never runs and `$h_new`'s
register is left holding whatever value it had before the scope was
entered — typically a transient RuntimeList left over from a temp
allocation in an unrelated statement whose register was later
recycled by `recycleTemporaryRegisters()`.
When the enclosing scope exits, SCOPE_EXIT_CLEANUP is emitted
unconditionally for every `my`-scalar declared in that scope. It
casts `registers[reg]` to `RuntimeScalar`. If the register holds the
stale RuntimeList, the cast fails with:
ClassCastException: RuntimeList cannot be cast to RuntimeScalar
## Why the fix is correct (not just a workaround)
The user never observes the stale value. The same short-circuit that
skipped the `my`-assignment also skipped the body that would have
read the variable — the "then" branch is guarded by the same
conditional that short-circuited. So at the language level, the
variable exists only in scopes where it was properly initialised.
`scopeExitCleanup()` only has real work to do on live RuntimeScalars:
IO-owner fd recycling for anonymous filehandles, refCount decrement
for blessed refs with DESTROY, and captureCount tracking for
closures. A non-scalar or null slot has no cleanup obligation; there
is nothing to decrement or close.
Verified against real code: DBI::PurePerl's dispatch wrapper at line
337-343 of upstream DBI 1.647 is exactly the short-circuit-my
pattern above, and it hits this path on every `connect()` call.
Before the fix, `t/09trace.t` was 1/99; after, 99/99.
## Testing
New regression test:
src/test/resources/unit/my_short_circuit_scope_exit.t
Covers three shapes:
- Direct sub with the short-circuit-my pattern, both paths taken
- eval-STRING compilation of the same pattern (matches DBI's path)
- 100-iteration loop to exercise register reuse
All three DBI dispatch variants of the real-world case also pass
after the fix:
- t/09trace.t 99/99 (was 1/99 at start of this PR)
- Entire DBI test suite unchanged at the Phase 9b numbers
## Follow-up (not in this PR)
Emitter-side cleanup: we could emit unconditional `LOAD_UNDEF` for
every my-scalar register at enterScope time (via a pre-pass over the
scope body to find all `my` declarations). That would eliminate the
stale-data window entirely. It's a larger refactor with no
user-visible benefit beyond defence-in-depth, so defer.
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1cdf092 to
3dd6e75
Compare
Summary
Lands Phase 8 (RootClass) plus the big architectural switch from our hand-rolled DBI.pm +
_Handles.pm(~2,300 lines) to upstream DBI 1.647 + DBI::PurePerl, with a thinDBD::JDBCbase driver so existing JDBC-backed DBDs (DBD::SQLite, DBD::Mem) keep working.6 commits, each standalone-reviewable:
a3b174be6DBI->connect({RootClass=>...})reblesses handles,_new_sthinherits error-handling attrs).fa2e618302d9479c68@ISA(e.g.$obj->Pkg::method()finds methods in@Pkg::ISAparents). Prerequisite for upstream DBI.pm.7f5a37156src/main/perl/lib/DBI.pmwith upstream DBI 1.647 (4-line PerlOnJava patch forcingDBI_PUREPERL=2). Addsrc/main/perl/lib/DBI/PurePerl.pm. DeleteDBI/_Handles.pm+DBI/_Utils.pm(PurePerl provides equivalents).b30e6cdf2DBD::JDBC::{dr,db,st}::*, newDBD::JDBCbase driver,DBD::SQLiterefactored to inherit from it.DBD::Memstub deleted (bundled upstream DBD::Mem takes over).b306e9e10Why
Every phase since Phase 1 has been re-implementing parts of DBI's XS API in pure Perl (
set_errseverity levels,HandleError,Callbacks,parse_trace_flags,_concat_hash_sorted,AutoCommitsentinels,RootClass, etc.). A Phase 9 spike confirmed that upstreamDBI.pm+DBI::PurePerlalready implement all of this and load under PerlOnJava with just a small patch. Switching to upstream stops our code from drifting away from the canonical DBI semantics.Per-test deltas (bundled DBI 1.647 test suite)
8 files go from partial-fail to full-pass; 4 more go from badly broken to ≥95%. Profile/Callbacks/Kids/swap_inner_handle are legitimately SKIPped by PurePerl — those are the XS-only features Phase 10 will reimplement in Java.
Known issues (follow-up PRs)
{ COND ? (%h1, %h2) : %h1 }under the JVM→interpreter fallback path. Not yet reproducible outside DBI.pm's connect closure context. Tracked in the plan doc; fix deferred.jcpan -t DBIbaseline not yet re-run. Per-test numbers extrapolate to ~5800–6300 passing subtests (from the 4940/6570 Phase 7 baseline).Phase 10 roadmap (Java shim for PurePerl-skipped XS features)
Test plan
makepasses (all unit tests)./jperl examples/dbi.plruns end-to-end via dbi:SQLite: (connect + do + prepare + execute + fetchrow_arrayref + fetchrow_hashref + disconnect)dev/modules/dbi_test_parity.md) with Phase 8/9/9b completion + Phase 10 roadmapjcpan -t DBIre-baseline (follow-up)Generated with Devin