Skip to content

Commit ebbb425

Browse files
committed
fix: address PR review comments for DBMS_OUTPUT
- ENABLE(NULL) now uses max buffer (1000000) instead of INT_MAX - Add named constants for buffer size limits and line allocation - Add XACT_EVENT_PARALLEL_COMMIT/ABORT handling - Remove references to external issue trackers (#22, #26) - Update test comments to reflect NULL buffer behavior
1 parent a6f814f commit ebbb425

File tree

3 files changed

+34
-31
lines changed

3 files changed

+34
-31
lines changed

contrib/ivorysql_ora/expected/ora_dbms_output.out

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,18 +242,18 @@ CALL dbms_output.enable(1000001);
242242
ERROR: buffer size must be between 2000 and 1000000
243243
CONTEXT: SQL statement "SELECT sys.ora_dbms_output_enable(buffer_size)"
244244
PL/iSQL function enable line 3 at PERFORM
245-
-- Test 4.5: NULL buffer size uses default
245+
-- Test 4.5: NULL buffer size uses maximum (1000000)
246246
DECLARE
247247
line TEXT;
248248
status INTEGER;
249249
BEGIN
250250
dbms_output.enable(NULL);
251-
dbms_output.put_line('NULL buffer uses default');
251+
dbms_output.put_line('NULL buffer uses max');
252252
dbms_output.get_line(line, status);
253253
RAISE NOTICE 'Test 4.5 - NULL buffer: [%]', line;
254254
END;
255255
/
256-
NOTICE: Test 4.5 - NULL buffer: [NULL buffer uses default]
256+
NOTICE: Test 4.5 - NULL buffer: [NULL buffer uses max]
257257
-- =============================================================================
258258
-- Section 5: Buffer overflow
259259
-- =============================================================================

contrib/ivorysql_ora/sql/ora_dbms_output.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,13 @@ END;
238238
-- Test 4.4: Buffer size above maximum (should fail)
239239
CALL dbms_output.enable(1000001);
240240

241-
-- Test 4.5: NULL buffer size uses default
241+
-- Test 4.5: NULL buffer size uses maximum (1000000)
242242
DECLARE
243243
line TEXT;
244244
status INTEGER;
245245
BEGIN
246246
dbms_output.enable(NULL);
247-
dbms_output.put_line('NULL buffer uses default');
247+
dbms_output.put_line('NULL buffer uses max');
248248
dbms_output.get_line(line, status);
249249
RAISE NOTICE 'Test 4.5 - NULL buffer: [%]', line;
250250
END;

contrib/ivorysql_ora/src/builtin_packages/dbms_output/dbms_output.c

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ static DbmsOutputBuffer *output_buffer = NULL;
6363
/* Oracle line length limit: 32767 bytes per line */
6464
#define DBMS_OUTPUT_MAX_LINE_LENGTH 32767
6565

66+
/* Buffer size limits (IvorySQL-enforced, stricter than Oracle) */
67+
#define DBMS_OUTPUT_MIN_BUFFER_SIZE 2000
68+
#define DBMS_OUTPUT_MAX_BUFFER_SIZE 1000000
69+
70+
/* Initial line array allocation parameters */
71+
#define DBMS_OUTPUT_MIN_LINES_ALLOCATED 100
72+
#define DBMS_OUTPUT_ESTIMATED_LINE_LENGTH 80
73+
6674
/* Internal function declarations */
6775
static void init_output_buffer(int buffer_size);
6876
static void cleanup_output_buffer(void);
@@ -87,7 +95,7 @@ PG_FUNCTION_INFO_V1(ora_dbms_output_get_lines);
8795
*
8896
* IvorySQL behavior: ENABLE always clears existing buffer.
8997
* Note: Oracle preserves buffer on re-ENABLE; this is an intentional
90-
* IvorySQL simplification. See GitHub issue #26 for tracking.
98+
* IvorySQL simplification.
9199
*/
92100
static void
93101
init_output_buffer(int buffer_size)
@@ -111,25 +119,16 @@ init_output_buffer(int buffer_size)
111119

112120
MemoryContextSwitchTo(output_buffer->buffer_mcxt);
113121

114-
/* Oracle tracks buffer in BYTES, not lines */
115-
if (buffer_size < 0)
116-
output_buffer->buffer_size = INT_MAX; /* NULL = unlimited (Oracle 10g R2+) */
117-
else
118-
output_buffer->buffer_size = buffer_size;
122+
/* Store buffer size in bytes (IvorySQL enforces 2000-1000000 range) */
123+
output_buffer->buffer_size = buffer_size;
119124

120125
/*
121126
* Pre-allocate line array with reasonable initial size.
122-
* For unlimited (INT_MAX), don't allocate huge array upfront -
123-
* we'll grow dynamically via ensure_lines_capacity().
127+
* Estimate based on average line length, with a minimum allocation.
124128
*/
125-
if (output_buffer->buffer_size == INT_MAX)
126-
output_buffer->lines_allocated = 1000; /* Start with 1000 lines for unlimited */
127-
else
128-
{
129-
output_buffer->lines_allocated = output_buffer->buffer_size / 80;
130-
if (output_buffer->lines_allocated < 100)
131-
output_buffer->lines_allocated = 100; /* Minimum array size */
132-
}
129+
output_buffer->lines_allocated = buffer_size / DBMS_OUTPUT_ESTIMATED_LINE_LENGTH;
130+
if (output_buffer->lines_allocated < DBMS_OUTPUT_MIN_LINES_ALLOCATED)
131+
output_buffer->lines_allocated = DBMS_OUTPUT_MIN_LINES_ALLOCATED;
133132

134133
output_buffer->lines = (char **) palloc0(sizeof(char *) * output_buffer->lines_allocated);
135134
output_buffer->current_line = makeStringInfo();
@@ -253,7 +252,9 @@ dbms_output_xact_callback(XactEvent event, void *arg)
253252
switch (event)
254253
{
255254
case XACT_EVENT_ABORT:
255+
case XACT_EVENT_PARALLEL_ABORT:
256256
case XACT_EVENT_COMMIT:
257+
case XACT_EVENT_PARALLEL_COMMIT:
257258
case XACT_EVENT_PREPARE:
258259
/* Clean up buffer at transaction end */
259260
cleanup_output_buffer();
@@ -269,31 +270,33 @@ dbms_output_xact_callback(XactEvent event, void *arg)
269270
* ora_dbms_output_enable
270271
*
271272
* Enable output buffering with optional size limit.
272-
* NULL parameter means UNLIMITED (Oracle 10g R2+).
273273
*
274-
* IvorySQL-enforced range: 2000 to 1000000 bytes when explicitly specified.
275-
* Note: Oracle silently clamps below-min values to 2000 and has no upper limit.
276-
* See GitHub issue #22 for tracking this difference.
274+
* IvorySQL-enforced range: 2000 to 1000000 bytes.
275+
* - NULL parameter uses maximum (1000000 bytes) for safety.
276+
* - Default (from SQL wrapper): 20000 bytes.
277277
*
278-
* Default (from SQL wrapper): 20000 bytes.
278+
* Note: Oracle allows unlimited buffer with NULL and silently clamps
279+
* below-min values to 2000. IvorySQL enforces stricter limits as a
280+
* protective measure for the initial implementation.
279281
*/
280282
Datum
281283
ora_dbms_output_enable(PG_FUNCTION_ARGS)
282284
{
283285
int32 buffer_size;
284286

285-
/* Handle NULL argument (means UNLIMITED) */
287+
/* Handle NULL argument - use maximum allowed size */
286288
if (PG_ARGISNULL(0))
287-
buffer_size = -1; /* -1 = unlimited */
289+
buffer_size = DBMS_OUTPUT_MAX_BUFFER_SIZE;
288290
else
289291
{
290292
buffer_size = PG_GETARG_INT32(0);
291293

292-
/* IvorySQL-enforced range (stricter than Oracle, see issue #22) */
293-
if (buffer_size < 2000 || buffer_size > 1000000)
294+
/* IvorySQL-enforced range (stricter than Oracle) */
295+
if (buffer_size < DBMS_OUTPUT_MIN_BUFFER_SIZE || buffer_size > DBMS_OUTPUT_MAX_BUFFER_SIZE)
294296
ereport(ERROR,
295297
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
296-
errmsg("buffer size must be between 2000 and 1000000")));
298+
errmsg("buffer size must be between %d and %d",
299+
DBMS_OUTPUT_MIN_BUFFER_SIZE, DBMS_OUTPUT_MAX_BUFFER_SIZE)));
297300
}
298301

299302
/* Initialize buffer (clears existing if present) */

0 commit comments

Comments
 (0)