-
Notifications
You must be signed in to change notification settings - Fork 156
Add Oracle-compatible DBMS_OUTPUT package implementation #998
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
base: master
Are you sure you want to change the base?
Conversation
Implement Oracle-compatible DBMS_OUTPUT package providing: - PUT_LINE, PUT, NEW_LINE for buffered output - GET_LINE, GET_LINES for retrieving output - ENABLE/DISABLE for buffer control - Buffer overflow detection (ORU-10027) Located in contrib/ivorysql_ora/src/builtin_packages/dbms_output/ following IvorySQL maintainer guidance (discussion IvorySQL#988). Includes design documentation and Oracle compatibility comparison with 82% test compatibility rate (27/33 tests pass). Known differences from Oracle: - NULL stored as empty string vs NULL - Re-ENABLE clears buffer vs preserves - Buffer size range 2000-1000000 (Oracle: 2000-unlimited) - No 32767 byte line length limit
Oracle treats empty strings as NULL. Both PUT_LINE('') and
PUT_LINE(NULL) should store actual NULL values, which GET_LINE
returns as SQL NULL.
Verified against Oracle 23.26 Free.
Fixes #25
Oracle enforces a maximum line length of 32767 bytes per line. Exceeding this limit raises ORU-10028 error. - Check line length in PUT_LINE before adding to buffer - Check accumulated line length in PUT before appending - Add test cases for line length boundary conditions Verified against Oracle 23.26 Free. Fixes #21
DBMS_OUTPUT Oracle Compatibility Test ResultsTest Date: 2025-12-03 Test MethodologyEquivalent test cases were executed on both IvorySQL and Oracle Database to verify behavioral compatibility. Tests use PUT followed by GET to verify actual buffer content rather than just syntax validation. Section 1: Basic PUT_LINE and GET_LINETest 1.1: Simple PUT_LINE verified by GET_LINEIvorySQL: Oracle: Result: ✅ MATCH Test 1.2: Multiple PUT_LINE calls verified by GET_LINESIvorySQL: Oracle: Result: ✅ MATCH Test 1.3: Empty string handlingIvorySQL: Oracle: Result: ✅ MATCH Analysis: Oracle treats empty strings as NULL. Both IvorySQL and Oracle store empty string as NULL and return it via GET_LINE. Test 1.4: NULL handlingIvorySQL: Oracle: Result: ✅ MATCH Analysis: Both IvorySQL and Oracle preserve NULL values. Fixed in commit c9aa53f (Fixes #25). Test 1.5: GET_LINE when buffer is emptyIvorySQL: Oracle: Result: ✅ MATCH Section 2: PUT and NEW_LINETest 2.1: PUT followed by NEW_LINEIvorySQL: Oracle: Result: ✅ MATCH Test 2.2: PUT with NULLIvorySQL: Oracle: Result: ✅ MATCH Test 2.3: Multiple PUT calls building one lineIvorySQL: Oracle: Result: ✅ MATCH Test 2.4: PUT + NEW_LINE + PUT_LINE creates two linesIvorySQL: Oracle: Result: ✅ MATCH Section 3: ENABLE and DISABLE behaviorTest 3.1: DISABLE prevents output from being bufferedIvorySQL: Oracle: Result: ✅ MATCH Test 3.2: DISABLE clears existing bufferIvorySQL: Oracle: Result: ✅ MATCH Test 3.3: Re-ENABLE clears bufferIvorySQL: Oracle: Result: ❌ DIFFERENCE Analysis: This is a significant behavioral difference:
This affects applications that call ENABLE() multiple times during execution. See Issue #26 for tracking. Test 3.4: Output while disabled is silently ignoredIvorySQL: Oracle: Result: ✅ MATCH Section 4: Buffer size limitsTest 4.1: Buffer size below minimum (1000)IvorySQL: Oracle: Result: ❌ DIFFERENCE Analysis: IvorySQL rejects values below 2000 with an error. Oracle silently adjusts values below 2000 up to 2000 (the minimum). See Issue #22 for tracking. Test 4.2: Buffer size at minimum (2000)IvorySQL: Oracle: Result: ✅ MATCH Test 4.3: Buffer size at maximum (1000000)IvorySQL: Oracle: Result: ✅ MATCH Test 4.4: Buffer size above maximum (1000001)IvorySQL: Oracle: Result: ❌ DIFFERENCE Analysis: IvorySQL enforces maximum buffer size of 1000000, Oracle accepts larger values. This is documented as an intentional IvorySQL limitation. Test 4.5: NULL buffer size uses defaultIvorySQL: Oracle: Result: ✅ MATCH Section 5: Buffer overflowTest 5.1: Buffer overflow produces errorIvorySQL: Oracle: Result: ✅ MATCH Analysis: Both produce the same Oracle-compatible error code (ORU-10027) and overflow occurs at approximately the same line count. Section 6: GET_LINE and GET_LINES behaviorTest 6.1: GET_LINE returns lines in orderIvorySQL: Oracle: Result: ✅ MATCH Test 6.2: GET_LINES with numlines larger than availableIvorySQL: Oracle: Result: ✅ MATCH Test 6.3: GET_LINES with numlines smaller than availableIvorySQL: Oracle: Result: ✅ MATCH Section 7: Usage in procedures and functionsTest 7.1: Output from procedureIvorySQL: Oracle: Result: ✅ MATCH Test 7.2: Output from functionIvorySQL: Oracle: Result: ✅ MATCH Section 8: Special casesTest 8.1: Special charactersIvorySQL: Oracle: Result: ✅ MATCH Test 8.2: Numeric values via concatenationIvorySQL: Oracle: Result: ✅ MATCH Test 8.3: Very long lineIvorySQL: Oracle: Result: ✅ MATCH Test 8.4: Exception handling preserves bufferIvorySQL: Oracle: Result: ✅ MATCH (error message format differs but behavior matches) Test 8.5: Nested blocksIvorySQL: Oracle: Result: ✅ MATCH Test 8.6: Loop outputIvorySQL: Oracle: Result: ✅ MATCH Section 9: Line Length LimitsTest 9.1: Maximum line length (32767 bytes)IvorySQL: Oracle: Result: ✅ MATCH Test 9.2: Exceeding max line length (32768 bytes via PUT_LINE)IvorySQL: Oracle: Result: ✅ MATCH Analysis: Both IvorySQL and Oracle enforce the 32767 byte line length limit with ORU-10028 error. Fixed in commit 80355d1 (Fixes #21). Test 9.3: PUT accumulating to exceed 32767 bytesIvorySQL: Oracle: Result: ✅ MATCH Analysis: Both IvorySQL and Oracle detect line length overflow during PUT accumulation. Fixed in commit 80355d1 (Fixes #21). Summary
Compatibility Rate: 91% (30/33 tests) Differences Summary
Fixed Issues
Recommendations
|
WalkthroughAdds an Oracle-compatible DBMS_OUTPUT feature to contrib/ivorysql_ora: new C implementation, SQL package/spec, regression tests, and build/manifest entries to compile and register the module. Changes
Sequence DiagramsequenceDiagram
autonumber
participant Client
participant SQL_Pkg as "SQL Package\n(dbms_output)"
participant C_Backend as "C Backend\n(ora_dbms_output_*)"
participant Buffer as "DbmsOutputBuffer"
participant Txn as "Txn Callback"
Client->>SQL_Pkg: ENABLE(size?)
SQL_Pkg->>C_Backend: ora_dbms_output_enable()
C_Backend->>Buffer: init_output_buffer()
Buffer->>Txn: register callback
Client->>SQL_Pkg: PUT(text) / PUT_LINE(text)
SQL_Pkg->>C_Backend: ora_dbms_output_put/_put_line
C_Backend->>Buffer: accumulate/append (enforce limits)
Client->>SQL_Pkg: NEW_LINE()
SQL_Pkg->>C_Backend: ora_dbms_output_new_line()
C_Backend->>Buffer: flush current PUT into lines array
Client->>SQL_Pkg: GET_LINE() / GET_LINES(n)
SQL_Pkg->>C_Backend: ora_dbms_output_get_line/get_lines
C_Backend->>Buffer: read from buffer (advance position)
Buffer-->>C_Backend: line(s) + status/count
C_Backend-->>SQL_Pkg: composite result
SQL_Pkg-->>Client: return values
Client->>SQL_Pkg: DISABLE()
SQL_Pkg->>C_Backend: ora_dbms_output_disable()
Txn->>Buffer: on commit/abort -> cleanup_output_buffer()
Buffer->>Txn: unregister callback / free memory
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
contrib/ivorysql_ora/sql/ora_dbms_output.sql (1)
147-189: Strong DBMS_OUTPUT coverage; consider clarifying DISABLE vs re‑ENABLE commentsThe tests around
DISABLE/re‑ENABLE(3.1–3.3) correctly verify that:
- Output is not buffered while disabled.
- A subsequent
ENABLEcall effectively resets the buffer.However, comments like “DISABLE clears existing buffer” in Test 3.2 can be misread as “DISABLE alone clears,” whereas the behavior is really “re‑ENABLE starts with a fresh buffer.” Consider tweaking those comments to avoid confusion for future readers; the test logic itself is fine.
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (1)
82-146: Minor behavioral/doc consistency around ENABLE/DISABLE and callback registrationTwo small nits:
The comment above
init_output_buffersays “Oracle behavior – ENABLE always clears existing buffer,” while your own tests intentionally validate a non‑Oracle behavior (re‑ENABLE clears, whereas Oracle typically preserves). It would be clearer to phrase this as “Our behavior: ENABLE always clears existing buffer (differs from Oracle)” to avoid confusion for future maintainers.Tracking
callback_registeredin the buffer struct and immediately unregistering insidecleanup_output_bufferworks, but given that the callback itself simply callscleanup_output_buffer(), you could simplify by:
- Keeping the callback permanently registered for the backend, and
- Letting it no‑op when
output_buffer == NULL.Not required for correctness, but it would reduce state bookkeeping and the chance of subtle registration/unregistration bugs later.
Also applies to: 296-309
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
contrib/ivorysql_ora/expected/ora_dbms_output.outis excluded by!**/*.outsrc/pl/plisql/src/expected/plisql_nested_subproc2.outis excluded by!**/*.out
📒 Files selected for processing (5)
contrib/ivorysql_ora/Makefile(2 hunks)contrib/ivorysql_ora/ivorysql_ora_merge_sqls(1 hunks)contrib/ivorysql_ora/sql/ora_dbms_output.sql(1 hunks)contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql(1 hunks)contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T10:43:25.257Z
Learnt from: syasirs
Repo: IvorySQL/IvorySQL PR: 858
File: src/Makefile.global.in:513-514
Timestamp: 2025-09-02T10:43:25.257Z
Learning: In IvorySQL Makefile.global.in, $(wildcard t/oracle/*.pl) is used instead of t/oracle/*.pl in Oracle TAP test discovery because the t/oracle directory is optional and may not exist in most builds. The wildcard function safely returns empty string when the directory is missing, preventing shell expansion errors.
Applied to files:
contrib/ivorysql_ora/Makefile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: meson_build (ubuntu-latest)
- GitHub Check: contrib_regression
- GitHub Check: oracle_regression (ubuntu-latest)
- GitHub Check: pg_regression (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (3)
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql (1)
26-127: DBMS_OUTPUT package spec/body and C wrappers look consistent and well‑aligned
- C functions, composite types, and package procedures line up correctly with the C signatures and intended OUT/IN OUT semantics.
- Using
VOLATILEfor these session/transaction‑scoped buffers is appropriate.
No issues from a correctness or integration standpoint.contrib/ivorysql_ora/ivorysql_ora_merge_sqls (1)
3-3: New dbms_output merge entry is consistent with file layoutThe added path
src/builtin_packages/dbms_output/dbms_outputmatches the new package’s location and naming convention; no further changes needed here.contrib/ivorysql_ora/Makefile (1)
28-29: Build and regression wiring for DBMS_OUTPUT looks correct
- Object file path
src/builtin_packages/dbms_output/dbms_output.omatches the new C source.ora_dbms_outputentry inORA_REGRESScorresponds to the new SQL test file and is placed consistently with existing tests.
No further Makefile changes appear necessary.Also applies to: 73-75
- Fix potential integer overflow in buffer check by using subtraction instead of addition (line_bytes > buffer_size - buffer_used) - Normalize negative numlines parameter to 0 in GET_LINES
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (2)
175-200: Consider guardinglines_allocatedgrowth against pathological overflow
ensure_lines_capacitygrowslines_allocatedby 50% (with a +100 floor), which is a good amortized strategy. In the “unlimited” case, it’s theoretically possible to push a huge number of zero-byte lines (e.g., repeatedNEW_LINEcalls with no text), causinglines_allocatedto grow very large. At extreme scales,new_capacity(anint) could overflow beforerepallocis called.This is extremely pathological, but you could defensively cap
new_capacity(e.g., check fornew_capacity <= output_buffer->lines_allocatedand throw an error) to avoid ever wrapping theint.
241-262: Optional: handle parallel xact events if IvorySQL’s base Postgres exposes them
dbms_output_xact_callbackcurrently cleans up only onXACT_EVENT_ABORT,XACT_EVENT_COMMIT, andXACT_EVENT_PREPARE, and ignores all other events. On newer Postgres versions there are alsoXACT_EVENT_PARALLEL_*variants commonly handled alongside the base events.If IvorySQL’s underlying Postgres has those events, you may want to route
XACT_EVENT_PARALLEL_COMMIT/PARALLEL_ABORT/PARALLEL_PREPAREthrough the same cleanup path to avoid leaking buffers in parallel contexts; if not, this can be left as-is.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (7)
src/backend/utils/mmgr/mcxt.c (6)
palloc0(1350-1370)MemoryContextDelete(453-488)pfree(1527-1541)repalloc(1547-1581)pstrdup(1702-1706)palloc(1320-1348)src/common/stringinfo.c (2)
makeStringInfo(71-75)resetStringInfo(125-134)src/backend/utils/fmgr/funcapi.c (1)
get_call_result_type(283-293)src/backend/executor/execTuples.c (1)
BlessTupleDesc(2314-2322)src/include/postgres.h (2)
Int32GetDatum(216-220)PointerGetDatum(326-330)src/include/funcapi.h (1)
HeapTupleGetDatum(336-340)src/backend/utils/adt/arrayfuncs.c (2)
construct_empty_array(3599-3610)construct_md_array(3513-3594)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: oracle_regression (ubuntu-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: oracle_pg_regression (ubuntu-latest)
- GitHub Check: meson_build (ubuntu-latest)
- GitHub Check: pg_regression (ubuntu-latest)
- GitHub Check: contrib_regression
🔇 Additional comments (5)
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (5)
39-57: Buffer struct design and memory-context split look solidThe
DbmsOutputBufferlayout (separatebuffer_mcxt, byte-basedbuffer_used,read_position, and preallocatedlinesarray) is clear and aligns well with Postgres memory management patterns. No issues here.
148-173: Cleanup and xact-callback unregistration pattern looks correct
cleanup_output_buffercorrectly:
- No-ops when
output_buffer == NULL,- Unregisters the xact callback only when it was registered,
- Deletes
buffer_mcxt(freeing all lines andcurrent_line), and- Frees the
DbmsOutputBufferstruct and resets the global pointer.This matches typical Postgres extension patterns; no changes needed.
202-238: Overflow check and NULL-line handling inadd_line_to_bufferlook goodThe revised overflow check using subtraction (
line_bytes > buffer_size - buffer_used) avoids signed-addition overflow, and treating NULL lines as zero bytes with a NULL pointer inlines[]matches the intended semantics. Allocating copies inbuffer_mcxtis also correct.No further issues here.
297-444: Core PUT/PUT_LINE/NEW_LINE semantics and limits look correctThe
ora_dbms_output_disable,ora_dbms_output_put_line,ora_dbms_output_put, andora_dbms_output_new_linepaths:
- Correctly ignore output when the buffer is disabled or not yet enabled.
- Enforce the 32767-byte per-line limit on both pure
PUT_LINEcalls and accumulatedPUTtext, raising ORU-10028 consistently.- Treat
PUT(NULL)as a no-op, andPUT_LINE(NULL)/NULL lines as actual NULL entries in the buffer.- Flush pending
PUTcontent beforePUT_LINE, and makeNEW_LINEcreate an empty line when there’s no pending text.Given the regression tests you described, these match the intended behavior; I don’t see any functional issues here.
446-590: GET_LINE/GET_LINES tuple and array construction looks correct; negativenumlineshandling is cleanFor
ora_dbms_output_get_lineandora_dbms_output_get_lines:
- The
get_call_result_type+BlessTupleDesc+heap_form_tuplepattern is standard and correct.- NULL lines are preserved (NULL in buffer → NULL element / field in result).
- Status codes and counts are consistent (
status = 1when no more lines, empty TEXT[] + count 0 when requested_lines ≤ 0 or buffer is empty).- Normalizing negative
requested_linesto 0 avoids surprising negative counts and aligns with the prior review feedback.No further changes needed here.
Address CodeRabbit review feedback: - Change "full Oracle compatibility" to "high Oracle compatibility" - Clarify that re-ENABLE clearing buffer is IvorySQL behavior (not Oracle) - Clarify that buffer size range is IvorySQL-enforced (stricter than Oracle) - Reference GitHub issues #22 and #26 for tracking differences
Add dbms_output_line and dbms_output_lines composite types to the expected output for the collation sanity check query.
|
@bigplaice following up #971, this one is real implementation. |
Overview
DBMS_OUTPUT is an Oracle-compatible package that provides a simple interface for displaying output from PL/SQL (PL/iSQL) blocks, stored procedures, functions, and triggers. It buffers text output during execution and allows retrieval via GET_LINE/GET_LINES procedures.
Architecture
Module Location
Design Decision: DBMS_OUTPUT is implemented within the
ivorysql_oraextension because:Extension ordering:
plisqlloads beforeivorysql_oraduring database initialization (seeinitdb.c). This allowsivorysql_orato usePACKAGEsyntax which requires PL/iSQL.Oracle package grouping: DBMS_OUTPUT belongs with other Oracle-compatible built-in packages in
ivorysql_ora.Type compatibility: Uses PostgreSQL native
TEXTtype instead ofVARCHAR2to avoid circular dependencies. Implicit casts between TEXT and VARCHAR2 ensure transparent compatibility.Component Diagram
Memory Management
TopMemoryContextfor session-level persistenceRegisterXactCallbackto clear buffer on transaction endListstructureStringInfoAPI Reference
ENABLE
Enables output buffering with specified buffer size.
Notes:
DISABLE
Disables output buffering and clears the buffer.
PUT
PROCEDURE put(a TEXT);Appends text to current line without newline.
Notes:
PUT_LINE
PROCEDURE put_line(a TEXT);Appends text and completes the line.
Notes:
NEW_LINE
Completes current line (adds newline to buffer).
GET_LINE
Retrieves one line from the buffer.
GET_LINES
Retrieves multiple lines from the buffer.
Implementation Details
Buffer Structure
Error Handling
Transaction Behavior
Test Coverage
Tests located in
contrib/ivorysql_ora/sql/ora_dbms_output.sqlTotal: 33 test cases
Oracle Compatibility
Comparison Summary
Compatibility Rate: 91% (30/33 tests pass)
Detailed Differences
1. Re-ENABLE Behavior
IvorySQL:
Oracle:
Impact: Medium. Applications that call ENABLE() multiple times may see different behavior.
2. Buffer Size Limits (#22)
IvorySQL: Enforces strict range 2000-1000000 bytes, rejects values outside.
Oracle: Minimum 2000 bytes (values below silently adjusted up), maximum unlimited. Per Oracle docs.
Impact: Low. IvorySQL is stricter but catches invalid values early.
Status: Open issue - should silently adjust values below 2000 instead of rejecting.
Compatibility Recommendations
For maximum compatibility:
Migration considerations:
Files Modified
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.ccontrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sqlcontrib/ivorysql_ora/Makefilecontrib/ivorysql_ora/meson.buildcontrib/ivorysql_ora/ivorysql_ora_merge_sqlsFuture Enhancements
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.