Add Oracle-compatible DBMS_OUTPUT package implementation

[rophy]

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

contrib/ivorysql_ora/
├── src/builtin_packages/dbms_output/
│   └── dbms_output.c           # C implementation
├── sql/ora_dbms_output.sql     # Test SQL
├── expected/ora_dbms_output.out # Expected test output
├── ivorysql_ora_merge_sqls     # SQL merge configuration
└── Makefile                    # Build configuration

Design Decision: DBMS_OUTPUT is implemented within the ivorysql_ora extension because:

1. Extension ordering: plisql loads before ivorysql_ora during database initialization (see initdb.c). This allows ivorysql_ora to use PACKAGE syntax which requires PL/iSQL.

2. Oracle package grouping: DBMS_OUTPUT belongs with other Oracle-compatible built-in packages in ivorysql_ora.

3. Type compatibility: Uses PostgreSQL native TEXT type instead of VARCHAR2 to avoid circular dependencies. Implicit casts between TEXT and VARCHAR2 ensure transparent compatibility.

Component Diagram

┌─────────────────────────────────────────────────────────────┐
│                      User Session                            │
├─────────────────────────────────────────────────────────────┤
│  PL/iSQL Block                                               │
│  ┌─────────────────────────────────────────────────────┐    │
│  │  dbms_output.put_line('Hello');                     │    │
│  │  dbms_output.get_line(line, status);                │    │
│  └─────────────────────────────────────────────────────┘    │
│                           │                                  │
│                           ▼                                  │
│  ┌─────────────────────────────────────────────────────┐    │
│  │  PACKAGE dbms_output (ivorysql_ora--1.0.sql)        │    │
│  │  - Wrapper procedures with Oracle-compatible API    │    │
│  └─────────────────────────────────────────────────────┘    │
│                           │                                  │
│                           ▼                                  │
│  ┌─────────────────────────────────────────────────────┐    │
│  │  C Functions (dbms_output.c)                        │    │
│  │  - ora_dbms_output_enable()                         │    │
│  │  - ora_dbms_output_put_line()                       │    │
│  │  - ora_dbms_output_get_line()                       │    │
│  │  - etc.                                             │    │
│  └─────────────────────────────────────────────────────┘    │
│                           │                                  │
│                           ▼                                  │
│  ┌─────────────────────────────────────────────────────┐    │
│  │  Session-level Buffer (TopMemoryContext)            │    │
│  │  - StringInfo for line buffer                       │    │
│  │  - List of completed lines                          │    │
│  │  - Buffer size tracking                             │    │
│  └─────────────────────────────────────────────────────┘    │
└─────────────────────────────────────────────────────────────┘

Memory Management

• Buffer Storage: Uses TopMemoryContext for session-level persistence
• Transaction Callbacks: Registered via RegisterXactCallback to clear buffer on transaction end
• Line Storage: Completed lines stored in a List structure
• Partial Line: Current incomplete line stored in StringInfo

API Reference

ENABLE

PROCEDURE enable(buffer_size INTEGER DEFAULT 20000);

Enables output buffering with specified buffer size.

Parameter	Type	Default	Description
buffer_size	INTEGER	20000	Buffer size in bytes (2000-1000000)

Notes:

• NULL buffer_size uses default (20000 bytes)
• Re-calling ENABLE clears existing buffer
• Buffer size below 2000 or above 1000000 raises error

DISABLE

PROCEDURE disable;

Disables output buffering and clears the buffer.

PUT

PROCEDURE put(a TEXT);

Appends text to current line without newline.

Parameter	Type	Description
a	TEXT	Text to append (NULL is ignored)

Notes:

• Accumulated line length must not exceed 32767 bytes
• Exceeding line length raises ORU-10028 error

PUT_LINE

PROCEDURE put_line(a TEXT);

Appends text and completes the line.

Parameter	Type	Description
a	TEXT	Text to output (NULL/empty string stored as NULL)

Notes:

• Maximum line length is 32767 bytes (Oracle compatible)
• Exceeding line length raises ORU-10028 error

NEW_LINE

PROCEDURE new_line;

Completes current line (adds newline to buffer).

GET_LINE

PROCEDURE get_line(line OUT TEXT, status OUT INTEGER);

Retrieves one line from the buffer.

Parameter	Direction	Type	Description
line	OUT	TEXT	Retrieved line (NULL if none)
status	OUT	INTEGER	0=success, 1=no more lines

GET_LINES

PROCEDURE get_lines(lines OUT TEXT[], numlines IN OUT INTEGER);

Retrieves multiple lines from the buffer.

Parameter	Direction	Type	Description
lines	OUT	TEXT[]	Array of retrieved lines
numlines	IN OUT	INTEGER	Requested/actual count

Implementation Details

Buffer Structure

typedef struct {
    bool        enabled;
    int         buffer_size;
    int         current_size;
    StringInfo  current_line;    /* Partial line being built */
    List       *lines;           /* Completed lines */
} DbmsOutputBuffer;

Error Handling

Error Code	Message	Condition
ORU-10027	buffer overflow, limit of N bytes	Total buffer size exceeded
ORU-10028	line length overflow, limit of 32767 bytes per line	Single line exceeds 32767 bytes
ERROR	buffer size must be between 2000 and 1000000	Invalid buffer_size parameter

Transaction Behavior

• Buffer persists across statements within a transaction
• Buffer is cleared on transaction commit/abort
• DISABLE clears buffer immediately

Test Coverage

Tests located in contrib/ivorysql_ora/sql/ora_dbms_output.sql

Section	Tests	Coverage
1. Basic PUT_LINE/GET_LINE	5	Content verification, empty/NULL handling, empty buffer status
2. PUT and NEW_LINE	4	Multi-PUT, NULL handling, line creation
3. ENABLE/DISABLE	4	Disable prevents buffering, clears buffer, re-enable behavior
4. Buffer size limits	5	Min/max bounds, error cases, NULL default
5. Buffer overflow	1	ORU-10027 error generation
6. GET behavior	3	FIFO order, partial retrieval, numlines adjustment
7. Procedures/Functions	2	Output preserved across proc/func calls
8. Special cases	6	Special chars, numerics, long lines, exceptions, nesting
9. Line length limits	3	32767 byte max, ORU-10028 error on overflow

Total: 33 test cases

Oracle Compatibility

Comparison Summary

Feature	IvorySQL	Oracle	Compatible
PUT_LINE basic	✓	✓	Yes
PUT + NEW_LINE	✓	✓	Yes
GET_LINE/GET_LINES	✓	✓	Yes
DISABLE behavior	✓	✓	Yes
Buffer overflow error	ORU-10027	ORU-10027	Yes
Line length limit	ORU-10028 at 32767	ORU-10028 at 32767	Yes
Proc/Func output	✓	✓	Yes
Exception handling	✓	✓	Yes
NULL/empty in PUT_LINE	Stored as NULL	Stored as NULL	Yes
Re-ENABLE behavior	Clears buffer	Preserves	No
Buffer size range	2000-1000000	2000-unlimited	No

Compatibility Rate: 91% (30/33 tests pass)

Detailed Differences

1. Re-ENABLE Behavior

IvorySQL:

dbms_output.enable();
dbms_output.put_line('First');
dbms_output.enable();  -- Clears buffer
dbms_output.get_line(line, status);
-- status = 1 (no lines)

Oracle:

DBMS_OUTPUT.ENABLE();
DBMS_OUTPUT.PUT_LINE('First');
DBMS_OUTPUT.ENABLE();  -- Preserves buffer
DBMS_OUTPUT.GET_LINE(line, status);
-- line = 'First', status = 0

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

1. For maximum compatibility:

   • Always call ENABLE() once at the start
   • Use buffer sizes within 2000-1000000
   • Keep individual lines under 32767 bytes

2. Migration considerations:

   • Audit code for multiple ENABLE() calls

Files Modified

File	Changes
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c	C implementation
contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql	Package SQL definition
contrib/ivorysql_ora/Makefile	Added dbms_output.o to OBJS
contrib/ivorysql_ora/meson.build	Added dbms_output.c to sources
contrib/ivorysql_ora/ivorysql_ora_merge_sqls	Added dbms_output SQL merge entry

Future Enhancements

1. SERVEROUTPUT setting: Add psql-like automatic output display
2. Re-ENABLE behavior: Consider matching Oracle's buffer-preserving behavior
3. Buffer size flexibility: Consider removing upper limit like Oracle

Summary by CodeRabbit

• New Features

  • Adds an Oracle-compatible DBMS_OUTPUT package with backend-buffered, transaction-scoped output. Supports ENABLE/DISABLE, PUT/PUT_LINE/NEW_LINE for emitting output and GET_LINE/GET_LINES for retrieval; preserves NULL semantics and enforces 32,767‑byte line limits.

• Tests

  • Adds comprehensive regression tests covering buffering, line assembly, enable/disable behavior, size limits, overflow, edge cases, and usage from procedures/functions.

• Chores

  • Includes the new DBMS_OUTPUT package in the extension build and regression run.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[rophy]

DBMS_OUTPUT Oracle Compatibility Test Results

Test Date: 2025-12-03
Oracle Version: 23.26.0.0 Free
IvorySQL Branch: feat/dbms_output

Test Methodology

Equivalent 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_LINE

Test 1.1: Simple PUT_LINE verified by GET_LINE

IvorySQL:

NOTICE:  Test 1.1 - Line: [Hello, World\!], Status: 0

Oracle:

Test 1.1 - Line: [Hello, World\!], Status: 0

Result: ✅ MATCH

---

Test 1.2: Multiple PUT_LINE calls verified by GET_LINES

IvorySQL:

NOTICE:  Test 1.2 - Retrieved 3 lines
NOTICE:    Line 1: [First line]
NOTICE:    Line 2: [Second line]
NOTICE:    Line 3: [Third line]

Oracle:

Test 1.2 - Retrieved 3 lines
  Line 1: [First line]
  Line 2: [Second line]
  Line 3: [Third line]

Result: ✅ MATCH

---

Test 1.3: Empty string handling

IvorySQL:

NOTICE:  Test 1.3 - Empty string: [<NULL>], Status: 0

Oracle:

Test 1.3 - Empty string: [<NULL>], Status: 0

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 handling

IvorySQL:

NOTICE:  Test 1.4 - NULL input: [<NULL>], Status: 0

Oracle:

Test 1.4 - NULL input: [<NULL>], Status: 0

Result: ✅ MATCH

Analysis: Both IvorySQL and Oracle preserve NULL values. Fixed in commit c9aa53f (Fixes #25).

---

Test 1.5: GET_LINE when buffer is empty

IvorySQL:

NOTICE:  Test 1.5 - Empty buffer: Line=[<NULL>], Status=1

Oracle:

Test 1.5 - Empty buffer: Line=[<NULL>], Status=1

Result: ✅ MATCH

---

Section 2: PUT and NEW_LINE

Test 2.1: PUT followed by NEW_LINE

IvorySQL:

NOTICE:  Test 2.1 - Combined: [First part second part], Status: 0

Oracle:

Test 2.1 - Combined: [First part second part], Status: 0

Result: ✅ MATCH

---

Test 2.2: PUT with NULL

IvorySQL:

NOTICE:  Test 2.2 - PUT with NULL: [BeforeAfter], Status: 0

Oracle:

Test 2.2 - PUT with NULL: [BeforeAfter], Status: 0

Result: ✅ MATCH

---

Test 2.3: Multiple PUT calls building one line

IvorySQL:

NOTICE:  Test 2.3 - Multiple PUTs: [ABCD], Status: 0

Oracle:

Test 2.3 - Multiple PUTs: [ABCD], Status: 0

Result: ✅ MATCH

---

Test 2.4: PUT + NEW_LINE + PUT_LINE creates two lines

IvorySQL:

NOTICE:  Test 2.4 - Retrieved 2 lines
NOTICE:    Line 1: [Partial]
NOTICE:    Line 2: [Complete]

Oracle:

Test 2.4 - Retrieved 2 lines
  Line 1: [Partial]
  Line 2: [Complete]

Result: ✅ MATCH

---

Section 3: ENABLE and DISABLE behavior

Test 3.1: DISABLE prevents output from being buffered

IvorySQL:

NOTICE:  Test 3.1 - After disable/enable cycle: [After re-enable], Status: 0

Oracle:

Test 3.1 - After disable/enable cycle: [After re-enable], Status: 0

Result: ✅ MATCH

---

Test 3.2: DISABLE clears existing buffer

IvorySQL:

NOTICE:  Test 3.2 - Buffer after disable: [<NULL>], Status: 1

Oracle:

Test 3.2 - Buffer after disable: [<NULL>], Status: 1

Result: ✅ MATCH

---

Test 3.3: Re-ENABLE clears buffer

IvorySQL:

NOTICE:  Test 3.3 - After re-enable: [<NULL>], Status: 1

Oracle:

Test 3.3 - After re-enable: [First enable content], Status: 0

Result: ❌ DIFFERENCE

Analysis: This is a significant behavioral difference:

• IvorySQL: Calling ENABLE() when already enabled clears the buffer
• Oracle: Calling ENABLE() when already enabled preserves existing buffer content

This affects applications that call ENABLE() multiple times during execution. See Issue #26 for tracking.

---

Test 3.4: Output while disabled is silently ignored

IvorySQL:

NOTICE:  Test 3.4 - Only visible after enable: [Visible], Status: 0

Oracle:

Test 3.4 - Only visible after enable: [Visible], Status: 0

Result: ✅ MATCH

---

Section 4: Buffer size limits

Test 4.1: Buffer size below minimum (1000)

IvorySQL:

ERROR:  buffer size must be between 2000 and 1000000

Oracle:

Test 4.1 - 1000 buffer: succeeded

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:

NOTICE:  Test 4.2 - Min buffer: [Min buffer works]

Oracle:

Test 4.2 - Min buffer: [Min buffer works]

Result: ✅ MATCH

---

Test 4.3: Buffer size at maximum (1000000)

IvorySQL:

NOTICE:  Test 4.3 - Max buffer: [Max buffer works]

Oracle:

Test 4.3 - Max buffer: [Max buffer works]

Result: ✅ MATCH

---

Test 4.4: Buffer size above maximum (1000001)

IvorySQL:

ERROR:  buffer size must be between 2000 and 1000000

Oracle:

Test 4.4 - 1000001 buffer: succeeded

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 default

IvorySQL:

NOTICE:  Test 4.5 - NULL buffer: [NULL buffer uses default]

Oracle:

Test 4.5 - NULL buffer: [NULL buffer uses default]

Result: ✅ MATCH

---

Section 5: Buffer overflow

Test 5.1: Buffer overflow produces error

IvorySQL:

NOTICE:  Test 5.1 - Overflow at line 47: ORU-10027: buffer overflow, limit of 2000 bytes

Oracle:

ORA-20000: ORU-10027: buffer overflow, limit of 2000 bytes
(Overflow at line 47)

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 behavior

Test 6.1: GET_LINE returns lines in order

IvorySQL:

NOTICE:  Test 6.1a - First: [Line A]
NOTICE:  Test 6.1b - Second: [Line B]
NOTICE:  Test 6.1c - Third: [Line C]
NOTICE:  Test 6.1d - Fourth (empty): [<NULL>], Status: 1

Oracle:

Test 6.1a - First: [Line A]
Test 6.1b - Second: [Line B]
Test 6.1c - Third: [Line C]
Test 6.1d - Fourth (empty): [<NULL>], Status: 1

Result: ✅ MATCH

---

Test 6.2: GET_LINES with numlines larger than available

IvorySQL:

NOTICE:  Test 6.2 - Requested 100, got 3
NOTICE:    Line 1: [Only]
NOTICE:    Line 2: [Three]
NOTICE:    Line 3: [Lines]

Oracle:

Test 6.2 - Requested 100, got 3
  Line 1: [Only]
  Line 2: [Three]
  Line 3: [Lines]

Result: ✅ MATCH

---

Test 6.3: GET_LINES with numlines smaller than available

IvorySQL:

NOTICE:  Test 6.3a - Got 2 lines with GET_LINES
NOTICE:    Line 1: [One]
NOTICE:    Line 2: [Two]
NOTICE:  Test 6.3b - Remaining: [Three], Status: 0
NOTICE:  Test 6.3c - Remaining: [Four], Status: 0

Oracle:

Test 6.3a - Got 2 lines with GET_LINES
  Line 1: [One]
  Line 2: [Two]
Test 6.3b - Remaining: [Three], Status: 0
Test 6.3c - Remaining: [Four], Status: 0

Result: ✅ MATCH

---

Section 7: Usage in procedures and functions

Test 7.1: Output from procedure

IvorySQL:

NOTICE:  Test 7.1 - From procedure: [Proc says: Hello from procedure]

Oracle:

Test 7.1 - From procedure: [Proc says: Hello from procedure]

Result: ✅ MATCH

---

Test 7.2: Output from function

IvorySQL:

NOTICE:  Test 7.2 - Function returned: 10
NOTICE:    Output 1: [Func input: 5]
NOTICE:    Output 2: [Func output: 10]

Oracle:

Test 7.2 - Function returned: 10
  Output 1: [Func input: 5]
  Output 2: [Func output: 10]

Result: ✅ MATCH

---

Section 8: Special cases

Test 8.1: Special characters

IvorySQL:

NOTICE:  Test 8.1 - Special chars: 3 lines
NOTICE:    [Tab:	here]
NOTICE:    [Quote: 'single' "double"]
NOTICE:    [Backslash: \ forward: /]

Oracle:

Test 8.1 - Special chars: 3 lines
  [Tab:	here]
  [Quote: 'single' "double"]
  [Backslash: \ forward: /]

Result: ✅ MATCH

---

Test 8.2: Numeric values via concatenation

IvorySQL:

NOTICE:  Test 8.2 - Numeric: [Number: 42]

Oracle:

Test 8.2 - Numeric: [Number: 42]

Result: ✅ MATCH

---

Test 8.3: Very long line

IvorySQL:

NOTICE:  Test 8.3 - Long line length: 1000

Oracle:

Test 8.3 - Long line length: 1000

Result: ✅ MATCH

---

Test 8.4: Exception handling preserves buffer

IvorySQL:

NOTICE:  Test 8.4a - [Before exception]
NOTICE:  Test 8.4b - [Caught: Test error]
NOTICE:  Test 8.4c - [After exception]

Oracle:

Test 8.4a - [Before exception]
Test 8.4b - [Caught: ORA-20001: Test error]
Test 8.4c - [After exception]

Result: ✅ MATCH (error message format differs but behavior matches)

---

Test 8.5: Nested blocks

IvorySQL:

NOTICE:  Test 8.5 - Nested blocks: 4 lines
NOTICE:    [Outer]
NOTICE:    [Inner 1]
NOTICE:    [Inner 2]
NOTICE:    [Back to outer]

Oracle:

Test 8.5 - Nested blocks: 4 lines
  [Outer]
  [Inner 1]
  [Inner 2]
  [Back to outer]

Result: ✅ MATCH

---

Test 8.6: Loop output

IvorySQL:

NOTICE:  Test 8.6 - Loop: 3 lines
NOTICE:    [Iteration 1]
NOTICE:    [Iteration 2]
NOTICE:    [Iteration 3]

Oracle:

Test 8.6 - Loop: 3 lines
  [Iteration 1]
  [Iteration 2]
  [Iteration 3]

Result: ✅ MATCH

---

Section 9: Line Length Limits

Test 9.1: Maximum line length (32767 bytes)

IvorySQL:

NOTICE:  Test 9.1 - Max line (32767 bytes): length=32767, Status=0

Oracle:

Test 9.1 (32767): OK, length=32767, Status=0

Result: ✅ MATCH

---

Test 9.2: Exceeding max line length (32768 bytes via PUT_LINE)

IvorySQL:

NOTICE:  Test 9.2 - Line overflow error: ORU-10028: line length overflow, limit of 32767 bytes per line

Oracle:

Test 9.2 (32768 via PUT): ORA-20000: ORU-10028: line length overflow, limit of 32767 bytes per line

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 bytes

IvorySQL:

NOTICE:  Test 9.3 - PUT overflow error: ORU-10028: line length overflow, limit of 32767 bytes per line

Oracle:

Test 9.3 (50000 via PUT): ORA-20000: ORU-10028: line length overflow, limit of 32767 bytes per line

Result: ✅ MATCH

Analysis: Both IvorySQL and Oracle detect line length overflow during PUT accumulation. Fixed in commit 80355d1 (Fixes #21).

---

Summary

Category	Tests	Passed	Failed
Section 1: Basic PUT_LINE/GET_LINE	5	5	0
Section 2: PUT and NEW_LINE	4	4	0
Section 3: ENABLE/DISABLE	4	3	1
Section 4: Buffer size limits	5	3	2
Section 5: Buffer overflow	1	1	0
Section 6: GET behavior	3	3	0
Section 7: Procedures/Functions	2	2	0
Section 8: Special cases	6	6	0
Section 9: Line length limits	3	3	0
Total	33	30	3

Compatibility Rate: 91% (30/33 tests)

Differences Summary

Test	Issue	Severity	Tracking
3.3	Re-ENABLE clears buffer vs preserves	Medium	Issue #26
4.1	Should silently adjust to 2000, not reject	Low	Issue #22
4.4	Maximum buffer 1000000 vs unlimited	Low	Documented as intentional limitation

Fixed Issues

Test	Issue	Fix
1.3, 1.4	NULL/empty string handling	Fixed in commit c9aa53f (#25)
9.2, 9.3	32767 byte line length limit	Fixed in commit 80355d1 (#21)

Recommendations

1. Medium Priority: Consider matching Oracle's re-ENABLE behavior (preserving buffer) as this could affect migrated applications. See Issue #26.

2. Low Priority: The buffer size differences are edge cases unlikely to affect most applications. See Issue #22.

3. Documentation: All remaining differences should be documented in user-facing documentation for migration guidance.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an Oracle-compatible DBMS_OUTPUT feature: new C backend implementing buffering and SQL-callable functions, PL/SQL package spec/body, tests, and build/merge entries to compile and register the module.

Changes

Cohort / File(s)	Summary
Build & merge contrib/ivorysql_ora/Makefile, contrib/ivorysql_ora/ivorysql_ora_merge_sqls	Add src/builtin_packages/dbms_output/dbms_output.o to OBJS; add src/builtin_packages/dbms_output/dbms_output to merge list; add ora_dbms_output to ORA_REGRESS.
C implementation contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c	New native implementation: per-backend transaction-scoped buffer, memory-context and txn-callback management, line accumulation, dynamic buffer growth, byte/line-size enforcement, and SQL-callable functions: enable/disable/put/put_line/new_line/get_line/get_lines.
SQL package contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql	Add PL/SQL package spec/body, composite return types, and declarations mapping PL/SQL procedures to the C functions.
Tests contrib/ivorysql_ora/sql/ora_dbms_output.sql	Add comprehensive regression tests covering PUT/PUT_LINE/NEW_LINE/GET_LINE/GET_LINES, ENABLE/DISABLE, buffer sizing, overflow, edge cases, nested blocks, session/transaction behavior, and helper procedures/functions.

Sequence Diagram

sequenceDiagram
    autonumber
    participant Client
    participant PL_SQL as "dbms_output (PL/SQL)"
    participant C_Backend as "C Backend\n(ora_dbms_output_*)"
    participant Buffer as "DbmsOutputBuffer"
    participant Txn as "Transaction Manager"

    Client->>PL_SQL: ENABLE(size?)
    PL_SQL->>C_Backend: ora_dbms_output_enable()
    C_Backend->>Buffer: init_output_buffer()
    Buffer->>Txn: register txn callback

    Client->>PL_SQL: PUT(text) / PUT_LINE(text)
    PL_SQL->>C_Backend: ora_dbms_output_put/_put_line
    C_Backend->>Buffer: accumulate/append (enforce limits)

    Client->>PL_SQL: NEW_LINE()
    PL_SQL->>C_Backend: ora_dbms_output_new_line()
    C_Backend->>Buffer: flush current PUT into lines array

    Client->>PL_SQL: GET_LINE() / GET_LINES(n)
    PL_SQL->>C_Backend: ora_dbms_output_get_line/get_lines
    C_Backend->>Buffer: read and advance position
    Buffer-->>C_Backend: line(s) + status/count
    C_Backend-->>PL_SQL: composite result
    PL_SQL-->>Client: return values

    Client->>PL_SQL: DISABLE()
    PL_SQL->>C_Backend: ora_dbms_output_disable()

    Txn->>Buffer: on commit/abort -> cleanup_output_buffer()
    Buffer->>Txn: unregister callback / free memory

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble at bytes and stitch each line,
ENABLE my burrow, PUT and then shine.
NEW_LINE hops in, GET_LINES brings cheer,
I stash each whisper till the commit is near. 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely summarizes the main change: adding an Oracle-compatible DBMS_OUTPUT package implementation to IvorySQL.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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 comments

The tests around DISABLE/re‑ENABLE (3.1–3.3) correctly verify that:

• Output is not buffered while disabled.
• A subsequent ENABLE call 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 registration

Two small nits:

• The comment above init_output_buffer says “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_registered in the buffer struct and immediately unregistering inside cleanup_output_buffer works, but given that the callback itself simply calls cleanup_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

📥 Commits

Reviewing files that changed from the base of the PR and between 7261f1c and ce711ad.

⛔ Files ignored due to path filters (2)

• contrib/ivorysql_ora/expected/ora_dbms_output.out is excluded by !**/*.out
• src/pl/plisql/src/expected/plisql_nested_subproc2.out is 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 VOLATILE for 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 layout

The added path src/builtin_packages/dbms_output/dbms_output matches 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.o matches the new C source.
• ora_dbms_output entry in ORA_REGRESS corresponds to the new SQL test file and is placed consistently with existing tests.
  No further Makefile changes appear necessary.

Also applies to: 73-75

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (2)

175-200: Consider guarding lines_allocated growth against pathological overflow

ensure_lines_capacity grows lines_allocated by 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., repeated NEW_LINE calls with no text), causing lines_allocated to grow very large. At extreme scales, new_capacity (an int) could overflow before repalloc is called.

This is extremely pathological, but you could defensively cap new_capacity (e.g., check for new_capacity <= output_buffer->lines_allocated and throw an error) to avoid ever wrapping the int.

---

241-262: Optional: handle parallel xact events if IvorySQL’s base Postgres exposes them

dbms_output_xact_callback currently cleans up only on XACT_EVENT_ABORT, XACT_EVENT_COMMIT, and XACT_EVENT_PREPARE, and ignores all other events. On newer Postgres versions there are also XACT_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_PREPARE through 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

📥 Commits

Reviewing files that changed from the base of the PR and between ce711ad and 74dd6d6.

📒 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 solid

The DbmsOutputBuffer layout (separate buffer_mcxt, byte-based buffer_used, read_position, and preallocated lines array) 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_buffer correctly:

• No-ops when output_buffer == NULL,
• Unregisters the xact callback only when it was registered,
• Deletes buffer_mcxt (freeing all lines and current_line), and
• Frees the DbmsOutputBuffer struct and resets the global pointer.

This matches typical Postgres extension patterns; no changes needed.

---

202-238: Overflow check and NULL-line handling in add_line_to_buffer look good

The 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 in lines[] matches the intended semantics. Allocating copies in buffer_mcxt is also correct.

No further issues here.

---

297-444: Core PUT/PUT_LINE/NEW_LINE semantics and limits look correct

The ora_dbms_output_disable, ora_dbms_output_put_line, ora_dbms_output_put, and ora_dbms_output_new_line paths:

• Correctly ignore output when the buffer is disabled or not yet enabled.
• Enforce the 32767-byte per-line limit on both pure PUT_LINE calls and accumulated PUT text, raising ORU-10028 consistently.
• Treat PUT(NULL) as a no-op, and PUT_LINE(NULL)/NULL lines as actual NULL entries in the buffer.
• Flush pending PUT content before PUT_LINE, and make NEW_LINE create 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; negative numlines handling is clean

For ora_dbms_output_get_line and ora_dbms_output_get_lines:

• The get_call_result_type + BlessTupleDesc + heap_form_tuple pattern is standard and correct.
• NULL lines are preserved (NULL in buffer → NULL element / field in result).
• Status codes and counts are consistent (status = 1 when no more lines, empty TEXT[] + count 0 when requested_lines ≤ 0 or buffer is empty).
• Normalizing negative requested_lines to 0 avoids surprising negative counts and aligns with the prior review feedback.

No further changes needed here.

[rophy]

@bigplaice following up #971, this one is real implementation.

[bigplaice]

The issue #22 here is not of IvorySQL. Suggest to delete this line.

[rophy]

Removed, was for rophy#22

[bigplaice]

wrong issue number?

[rophy]

Removed

[bigplaice]

The default is unlimited when argument is null, not 20000.

[rophy]

Changed NULL to 1000000 to align with the scope of this PR.

[bigplaice]

I believe that issue #26 is from somewhere else instead of IvorySQL.

[rophy]

Removed, was meant to refer rophy#26

[bigplaice]

it would be better to define macros for value 80 and 100.

[bigplaice]

Same comment for the min/max size of buffer, i.e. 2000 and 1000000

[rophy]

fixed

[bigplaice]

I think we should add XACT_EVENT_PARALLEL_COMMIT/XACT_EVENT_PARALLEL_ABORT, just like what plpgsql_xact_cb() does.

[rophy]

fixed

[yuanyl630]

Thanks @rophy for this comprehensive DBMS_OUTPUT implementation! The code quality is good with clear structure, proper memory management, and thorough test coverage.

I've identified a few issues that need to be addressed before merging:
1. Buffer Scope Behavior
The current implementation uses transaction-scoped buffer, but Oracle uses session-scoped buffer. I verified this on Oracle 21c - see detailed comment in dbms_output.c
2. OUT Parameter Type Compatibility
The package uses TEXT type:
PROCEDURE get_line(line OUT TEXT, status OUT INTEGER);
However, when users declare variables as VARCHAR2 (common Oracle practice):
DECLARE
v_line VARCHAR2(32767);
v_status INTEGER;
BEGIN
DBMS_OUTPUT.GET_LINE(v_line, v_status); -- ERROR!
END;

Oracle's Signature:
PROCEDURE GET_LINE (line OUT VARCHAR2, status OUT INTEGER);

Although IvorySQL has implicit casts between TEXT and VARCHAR2, these casts don't work for OUT parameters because the value cannot be "written back" through the cast.

Suggest to add overloaded procedures for both TEXT and VARCHAR2 in dbms_output--1.0.sql,
-- Create DBMS_OUTPUT package
CREATE OR REPLACE PACKAGE dbms_output IS
PROCEDURE enable(buffer_size INTEGER DEFAULT 20000);
PROCEDURE disable;
PROCEDURE put_line(a TEXT);

• PROCEDURE put_line(a VARCHAR2); //not necessary, implicit casts can handle varchar2 argument
  PROCEDURE put(a TEXT);
• PROCEDURE put(a VARCHAR2); //not necessary, implicit casts can handle varchar2 argument
  PROCEDURE new_line;
  PROCEDURE get_line(line OUT TEXT, status OUT INTEGER);
• PROCEDURE get_line(line OUT VARCHAR2, status OUT INTEGER);
  PROCEDURE get_lines(lines OUT TEXT[], numlines IN OUT INTEGER);
• PROCEDURE get_lines(lines OUT VARCHAR2[], numlines IN OUT INTEGER);
  END dbms_output;

[yuanyl630]

In Oracle, a message buffer is local to a single session. That means the buffer is session-scoped. Following test can be verified on Oracle 21c. The test result is "Read after COMMIT: Before COMMIT", which confirms the buffer is session-scoped instead of transaction-scoped.

SET SERVEROUTPUT OFF

-- 1. Test: What happens to buffer content after COMMIT?
BEGIN
DBMS_OUTPUT.ENABLE(1000000);
DBMS_OUTPUT.PUT_LINE('Before COMMIT');
END;
/

COMMIT;

SET SERVEROUTPUT ON

-- 2. Read the buffer content and display it.
DECLARE
v_line VARCHAR2(32767);
v_status INTEGER;
BEGIN
DBMS_OUTPUT.GET_LINE(v_line, v_status);
IF v_status = 0 THEN
DBMS_OUTPUT.PUT_LINE('Read after COMMIT: ' || v_line);
ELSE
DBMS_OUTPUT.PUT_LINE('Buffer is empty after COMMIT');
END IF;
END;
/

The comment needs to be corrected.

Oracle behavior: buffer is transaction-scoped, not session-scoped.

[yuanyl630]

When SET SERVEROUTPUT ON is enabled in SQLPlus, the client automatically calls DBMS_OUTPUT.GET_LINES() after each PL/SQL block execution to retrieve and display all buffered output. This means the buffer is effectively cleared after each block, which may affect cross-block buffer persistence testing.

[rophy]

Commit 6245ed2 changes buffer to session scope, commit c647a68 has test cases which fails before fix, and passes after fix.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a0b0369 and ebbb425.

⛔ Files ignored due to path filters (3)

• contrib/ivorysql_ora/expected/ora_dbms_output.out is excluded by !**/*.out
• src/oracle_test/regress/expected/opr_sanity.out is excluded by !**/*.out
• src/test/regress/expected/opr_sanity_1.out is excluded by !**/*.out

📒 Files selected for processing (2)

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:264-295
Timestamp: 2025-12-03T06:42:16.044Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the buffer size validation (2000–1000000 bytes) is an intentional IvorySQL-enforced constraint that diverges from Oracle behavior. The minimum constraint behavior is tracked in issue #22, and the maximum limit discussion is deferred as future work per rophy's conservative approach for initial implementation.

📚 Learning: 2025-12-03T06:42:16.044Z

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:264-295
Timestamp: 2025-12-03T06:42:16.044Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the buffer size validation (2000–1000000 bytes) is an intentional IvorySQL-enforced constraint that diverges from Oracle behavior. The minimum constraint behavior is tracked in issue #22, and the maximum limit discussion is deferred as future work per rophy's conservative approach for initial implementation.

Applied to files:

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

📚 Learning: 2025-12-03T06:50:27.755Z

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

Applied to files:

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

🧬 Code graph analysis (1)

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (10)

src/include/utils/palloc.h (1)

• MemoryContextSwitchTo (123-130)

src/backend/utils/mmgr/mcxt.c (3)

• palloc0 (1350-1370)
• pfree (1527-1541)
• palloc (1320-1348)

src/backend/access/transam/xact.c (2)

• RegisterXactCallback (3804-3815)
• UnregisterXactCallback (3817-3836)

src/backend/utils/error/elog.c (2)

• errcode (853-864)
• errmsg (1070-1086)

src/backend/utils/adt/varlena.c (1)

• text_to_cstring (226-242)

src/backend/executor/execTuples.c (1)

• BlessTupleDesc (2314-2322)

src/include/postgres.h (2)

• Int32GetDatum (216-220)
• PointerGetDatum (326-330)

src/backend/access/common/heaptuple.c (1)

• heap_form_tuple (1149-1235)

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: meson_build (ubuntu-latest)
• GitHub Check: oracle_pg_regression (ubuntu-latest)
• GitHub Check: oracle_regression (ubuntu-latest)
• GitHub Check: contrib_regression
• GitHub Check: build (ubuntu-latest)
• GitHub Check: pg_regression (ubuntu-latest)

🔇 Additional comments (2)

contrib/ivorysql_ora/sql/ora_dbms_output.sql (1)

1-549: Comprehensive test coverage with clear structure.

The test suite provides excellent coverage of DBMS_OUTPUT functionality across 9 well-organized sections with 33 test cases. The design principle of verifying every PUT with a corresponding GET operation ensures proper validation of buffering behavior.

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (1)

1-601: Well-implemented Oracle-compatible DBMS_OUTPUT package.

The C implementation demonstrates solid engineering:

• Proper memory management using dedicated memory contexts
• Correct transaction callback lifecycle management
• Oracle-compatible error handling (ORU-10027, ORU-10028)
• Faithful NULL semantics and line length enforcement (32767 bytes)
• All previously identified issues (overflow checks, negative numlines handling) have been addressed

The documented intentional differences from Oracle (buffer size validation and re-ENABLE behavior) are clearly commented and tracked in issues #22 and #26.

[yuanyl630]

Another issue: Buffer memory is not recycled

Currently, GET_LINE/GET_LINES only advances read_position but does not:

• Free the memory of already-read lines
• Decrease buffer_used
• Decrease line_count

This means:

1. Memory usage only grows, never shrinks (until buffer cleanup)
2. buffer_used counts all written bytes, even if already consumed
3. Users may hit ORU-10027 buffer overflow even though read data "should" free up space

Consider implementing immediate memory release on read or ring buffer is better. Any thought? :)

[yuanyl630]

This memory management issue may become worse when using enable(NULL). Currently enable(NULL) could allocate at most 2GB memory for debugging information output. This may bring safty risk to the backend process. Suggest changing 2GB to a smaller value. How about 200MB?

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ebbb425 and 42afdcc.

⛔ Files ignored due to path filters (1)

• contrib/ivorysql_ora/expected/ora_dbms_output.out is excluded by !**/*.out

📒 Files selected for processing (3)

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

🚧 Files skipped from review as they are similar to previous changes (1)

• contrib/ivorysql_ora/sql/ora_dbms_output.sql

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:264-295
Timestamp: 2025-12-03T06:42:16.044Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the buffer size validation (2000–1000000 bytes) is an intentional IvorySQL-enforced constraint that diverges from Oracle behavior. The minimum constraint behavior is tracked in issue #22, and the maximum limit discussion is deferred as future work per rophy's conservative approach for initial implementation.

📚 Learning: 2025-12-03T06:50:27.755Z

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

Applied to files:

• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

📚 Learning: 2025-12-03T06:42:16.044Z

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:264-295
Timestamp: 2025-12-03T06:42:16.044Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the buffer size validation (2000–1000000 bytes) is an intentional IvorySQL-enforced constraint that diverges from Oracle behavior. The minimum constraint behavior is tracked in issue #22, and the maximum limit discussion is deferred as future work per rophy's conservative approach for initial implementation.

Applied to files:

• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

⏰ 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_pg_regression (ubuntu-latest)
• GitHub Check: pg_regression (ubuntu-latest)
• GitHub Check: contrib_regression
• GitHub Check: oracle_regression (ubuntu-latest)
• GitHub Check: meson_build (ubuntu-latest)
• GitHub Check: build (ubuntu-latest)

🔇 Additional comments (3)

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (1)

18-23: Documentation accurately reflects implementation scope.

The header correctly describes the implementation as providing "high Oracle compatibility" (not "full") and directs readers to the test suite for behavioral differences. This addresses previous feedback about clarity.

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output--1.0.sql (2)

26-30: Default buffer size mismatch with implementation intent.

The SQL wrapper defines the default as 20000 bytes (line 27), but the C implementation documents at lines 230-232 that NULL should map to the maximum (1000000 bytes) for safety. However, line 244-245 in the C code shows NULL actually does map to DBMS_OUTPUT_MAX_BUFFER_SIZE (1000000).

The issue: when callers use the SQL default (omit the argument), they get 20000, not 1000000. This means:

• DBMS_OUTPUT.ENABLE() → 20000 bytes (SQL default)
• DBMS_OUTPUT.ENABLE(NULL) → 1000000 bytes (C NULL handling)

If the intent is for the default to be 1000000 (as suggested by the C code comments and the PR objective to reduce the default for NULL to 200MB in future work), consider changing line 27 to DEFAULT NULL so the C function handles it.

Alternatively, if 20000 is the intentional default and NULL is just for "maximum when explicitly requested", the C comment at lines 231-232 should be updated to clarify this distinction.

---

113-143: OUT parameter wrapper implementation is correct.

The package body properly implements OUT parameters for GET_LINE and GET_LINES by:

1. Calling the C function via SELECT INTO a local composite variable
2. Assigning the composite fields to the OUT parameters

The VARCHAR2 overloads (lines 121-127, 137-143) correctly reuse the same C functions, relying on PostgreSQL's implicit TEXT↔VARCHAR2 compatibility. This addresses the OUT parameter compatibility concern raised in the PR objectives.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (1)

54-66: Consider potential overflow of head/tail pointers in extreme long-running sessions.

The ring buffer uses int for head and tail pointers (lines 58-59), which are continuously incremented (lines 419, 670, 762) without resetting. While wrap-around is handled via modulo during access (line 287), the pointers themselves could theoretically overflow INT_MAX (~2.1B) in an extremely long-running session with massive data throughput.

This is a minor theoretical concern with negligible real-world impact (would require writing/reading ~2GB worth of buffer operations), but for defensive robustness you could consider:

• Using int64 for head/tail to make overflow practically impossible
• Or periodically normalizing head/tail (subtract tail from both) when the buffer is empty

Given the extremely low likelihood and the complexity of normalization, this is optional.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42afdcc and 5fad8c8.

⛔ Files ignored due to path filters (1)

• contrib/ivorysql_ora/expected/ora_dbms_output.out is excluded by !**/*.out

📒 Files selected for processing (2)

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:264-295
Timestamp: 2025-12-03T06:42:16.044Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the buffer size validation (2000–1000000 bytes) is an intentional IvorySQL-enforced constraint that diverges from Oracle behavior. The minimum constraint behavior is tracked in issue #22, and the maximum limit discussion is deferred as future work per rophy's conservative approach for initial implementation.

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

📚 Learning: 2025-12-03T06:42:16.044Z

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:264-295
Timestamp: 2025-12-03T06:42:16.044Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the buffer size validation (2000–1000000 bytes) is an intentional IvorySQL-enforced constraint that diverges from Oracle behavior. The minimum constraint behavior is tracked in issue #22, and the maximum limit discussion is deferred as future work per rophy's conservative approach for initial implementation.

Applied to files:

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

📚 Learning: 2025-12-03T06:50:27.755Z

Learnt from: rophy
Repo: IvorySQL/IvorySQL PR: 998
File: contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c:82-146
Timestamp: 2025-12-03T06:50:27.755Z
Learning: In contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c, the behavior where ENABLE() clears the existing buffer (rather than preserving it like Oracle does) is an intentional known difference tracked in issue #26 at https://github.com/rophy/IvorySQL/issues/26 (medium impact, open).

Applied to files:

• contrib/ivorysql_ora/sql/ora_dbms_output.sql
• contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

⏰ 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_pg_regression (ubuntu-latest)
• GitHub Check: meson_build (ubuntu-latest)
• GitHub Check: oracle_regression (ubuntu-latest)
• GitHub Check: build (ubuntu-latest)
• GitHub Check: pg_regression (ubuntu-latest)
• GitHub Check: contrib_regression

🔇 Additional comments (14)

contrib/ivorysql_ora/sql/ora_dbms_output.sql (1)

1-719: Comprehensive test suite with excellent coverage.

The test file is well-structured with 12 sections covering basic operations, edge cases, buffer limits, overflow scenarios, session scope, VARCHAR2 compatibility, and buffer space recycling. The tests align with the implementation's known behavioral differences from Oracle (re-ENABLE clearing buffer, buffer size constraints) and validate the critical ring buffer recycling behavior.

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c (13)

18-28: LGTM - Header documentation is accurate.

The header correctly describes session-level buffering and appropriately states "high Oracle compatibility" with a reference to tests for known differences. This addresses the previous feedback about avoiding "full compatibility" claims.

---

106-170: LGTM - Initialization logic is sound.

The function correctly implements the intentional IvorySQL behavior (clearing buffer on re-ENABLE) with clear documentation. Memory context management is appropriate, and the initial capacity strategy (DEFAULT_BUFFER_SIZE for unlimited, full size for fixed) is efficient.

Based on learnings: The ENABLE clearing behavior is a known intentional difference tracked in issue #26.

---

172-190: LGTM - Cleanup is correct.

The cleanup logic properly deletes the memory context (which automatically frees buffer and current_line) and resets the global pointer.

---

192-277: LGTM - Buffer expansion and packing logic is correct.

The dynamic growth strategy is well-designed, and the ring buffer linearization correctly handles both contiguous and wrapped cases. The growth factor ensures reasonable allocation behavior.

---

279-354: LGTM - Ring buffer operations correctly handle wrap-around.

All ring buffer read/write functions properly handle wrap-around cases, and uint16 encoding/decoding is consistent with little-endian byte order.

---

367-422: LGTM - Line addition logic is correct with proper overflow protection.

The function correctly checks user-perceived limits using safe subtraction (avoiding integer overflow), expands internal storage when needed, and properly updates all accounting fields. The ring buffer space recycling is implemented (buffer_used is decremented in GET functions).

---

424-456: LGTM - ENABLE function correctly implements Oracle-compatible behavior.

The function properly handles NULL (unlimited) and silently clamps below-minimum values to 2000 bytes, matching Oracle behavior.

Based on learnings: The buffer size constraints are intentional IvorySQL design choices tracked in issue #22.

---

458-471: LGTM - DISABLE function is correct.

The function correctly implements Oracle behavior: disables buffering but preserves buffer content for subsequent GET operations.

---

473-536: LGTM - PUT_LINE function correctly implements Oracle semantics.

The function properly handles NULL arguments, pending PUT text, and enforces the 32767-byte line length limit with appropriate error codes. The logic for combining pending PUT text with PUT_LINE is correct.

---

538-578: LGTM - PUT function correctly accumulates text without creating lines.

The function properly handles NULL (treats as empty string), checks line length limits before appending, and accumulates text in current_line until NEW_LINE or PUT_LINE is called.

---

580-607: LGTM - NEW_LINE function correctly flushes accumulated PUT text.

The function properly flushes current_line to the buffer and creates empty lines when no PUT text was accumulated, matching Oracle behavior.

---

609-676: LGTM - GET_LINE function correctly implements buffer space recycling.

The function properly reads lines from the ring buffer and crucially decrements buffer_used and advances the tail pointer, allowing buffer space to be recycled for new writes (Oracle behavior). This addresses the memory reclamation concern raised in PR feedback.

---

678-783: LGTM - GET_LINES function correctly implements batch retrieval with recycling.

The function properly handles negative numlines (normalizes to 0), reads multiple lines efficiently, implements buffer space recycling, and constructs 1-based arrays for Oracle compatibility.

[rophy]

Thanks for all the professional feedbacks. Commit 5fad8c8 changed to session-memory, removed the 100000 bytes limit. implemented ring buffer, and implemented ENABLE(NULL) unlimited expansion.

I tried to test how oracle allocates memory. It seems oracle actually implemented some kind of linked list for dynamic memory allocation, while the implementation in this PR is different. A detailed report is here: rophy#29

With latest commit:

• Each log line is length-prefixed. A singed integer of 2 bytes. At worst case that every line is single byte, actual memory would be 3x user specified buffer size
• ENABLE(NULL) will have actual_buffer_size starts as 20000 bytes, and expands by max(20000, capacity * 20%)
• ENABLE(specified_buffer_size) will have actual_buffer_size starts as specified_buffer_size, and expands by min(specified_buffer_size, max(DEFAULT_BUFFER_SIZE, capacity * GROWTH_FACTOR))
• Multiple calls to ENABLE() is still not fixed (ivorysql clears buffer, oracle resets buffer). This is actually kind of complicated (Oracle DBMS_OUTPUT: Multiple ENABLE() Calls Behavior rophy/IvorySQL#30) so I want to leave that to future PRs.

[OreoYang]

Hello @rophy ,
Since we introducing new .c file contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c.
Could you please ensure it works properly during Meson builds?

[bigplaice]

no comments.

[bigplaice]

Hi, Rophy, session-level buffering is more reasonable and compatible with Oracle. However, this patch implements transaction-level buffer. Is there some reason to do so?

[bigplaice]

we need release the memory of line_str here.

[bigplaice]

the brace can be removed.

[bigplaice]

Maybe we should consider the case of no input argument, and give requested_lines a default value?