feat: add user-defined EXCEPTION support for PL/iSQL#972
feat: add user-defined EXCEPTION support for PL/iSQL#972gaoxueyu merged 4 commits intoIvorySQL:masterfrom
Conversation
Implement Oracle-compatible user-defined exception variables in PL/iSQL.
Exceptions can be declared in packages and procedures, raised with RAISE,
and caught with WHEN handlers.
Example:
CREATE PACKAGE BODY pkg IS
custom_error EXCEPTION;
PROCEDURE test IS
BEGIN
RAISE custom_error;
EXCEPTION
WHEN custom_error THEN
RAISE INFO 'Caught exception';
END;
END;
Implementation stores exceptions as datums with unique dno, registered
in namespace for lookup. Default SQLCODE is 1 for user-defined exceptions.
WalkthroughThis pull request introduces user-defined exception support to PL/iSQL. The changes add a new PLISQL_DTYPE_EXCEPTION datum type, implement exception variable creation and management, extend the parser to recognize EXCEPTION declarations and PRAGMA EXCEPTION_INIT statements, and integrate exception handling throughout compilation and runtime execution paths. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Parser as Parser/Lexer<br/>(pl_gram.y)
participant Compiler as Compiler<br/>(pl_comp.c)
participant Exception as Exception System<br/>(pl_exception_type.c)
participant Namespace as Namespace<br/>(Lookup)
participant Runtime as Runtime<br/>(pl_exec.c)
User->>Parser: DECLARE exc_custom EXCEPTION;
Parser->>Compiler: decl_varname K_EXCEPTION
Compiler->>Exception: plisql_build_exception(refname, lineno, add2namespace)
Exception->>Namespace: Register exception variable
Namespace-->>Exception: datum registered
Exception-->>Compiler: PLiSQL_exception_var created
rect rgb(200, 220, 255)
Note over User,Compiler: Optional: PRAGMA EXCEPTION_INIT
User->>Parser: PRAGMA EXCEPTION_INIT(exc_custom, -1000)
Parser->>Compiler: K_PRAGMA K_EXCEPTION_INIT
Compiler->>Exception: plisql_process_pragma_exception_init(name, code, ...)
Exception->>Exception: Validate Oracle-compatible error code
Exception-->>Compiler: SQLCODE assigned to exception
end
User->>Runtime: RAISE exc_custom;
Runtime->>Namespace: plisql_lookup_exception(exc_name)
Namespace-->>Runtime: Return PLiSQL_exception_var with SQLCODE
Runtime->>Runtime: Use exception metadata (SQLCODE, refname) for error handling
Runtime-->>User: Exception raised/caught
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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
~/IvorySQL/src/pl/plisql$ make oracle-check
......
# +++ ora regress check in src/pl/plisql/src +++
# initializing database system by copying initdb template
# using temp instance on port 65313 with PID 62850
ok 1 - plisql_array 17 ms
ok 2 - plisql_call 34 ms
ok 3 - plisql_control 25 ms
ok 4 - plisql_copy 11 ms
ok 5 - plisql_domain 24 ms
ok 6 - plisql_record 61 ms
ok 7 - plisql_cache 11 ms
ok 8 - plisql_simple 16 ms
ok 9 - plisql_transaction 49 ms
ok 10 - plisql_trap 1043 ms
ok 11 - plisql_trigger 10 ms
ok 12 - plisql_varprops 11 ms
ok 13 - plisql_nested_subproc 14986 ms
ok 14 - plisql_nested_subproc2 15031 ms
ok 15 - plisql_out_parameter 67 ms
ok 16 - plisql_type_rowtype 539 ms
ok 17 - plisql_exception 9 ms
1..17
# All 17 tests passed.
make[1]: Leaving directory '/home/ivorysql/IvorySQL/src/pl/plisql/src' |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
src/pl/plisql/src/plisql.h (1)
67-74: NewPLISQL_DTYPE_EXCEPTIONenum value fits the existing datum modelAdding
PLISQL_DTYPE_EXCEPTIONtoPLiSQL_datum_typeis consistent with how other datum kinds are modelled. The surrounding code in this PR (freeing, dumptree, const-ness checks) already recognizes this new value, so the enum extension is coherent. Just ensure any remainingswitch (dtype)sites in other modules have been audited for this new alternative.src/pl/plisql/src/pl_comp.c (3)
2814-2881: HandlingPLISQL_DTYPE_EXCEPTIONinbuild_row_from_varsmay merit an explicit design commentMapping exception variables to a
VOIDattribute in the row tuple descriptor avoids the generic “unrecognized dtype” error and keeps exception vars from being treated as a normal SQL type. However, this is a bit of a special case:
- If a row built from OUT vars is ever exposed to SQL or client code that inspects its
TupleDesc, aVOIDcolumn may be surprising.- If exception vars are never supposed to participate in observable row values, an
Assert(false)(or at least a comment stating “this case should not be hit for user-visible rows”) would make intent clearer.Consider either:
- Documenting here that exception vars are internal-only and that the
VOIDmapping is never surfaced, or- Guarding with an assertion in debug builds to catch accidental uses where an exception var is stuffed into a row that will be materialized.
3267-3282: Confirm whetherPLISQL_DTYPE_EXCEPTIONneeds to participate incopiable_size
plisql_finish_datumsstill accounts only forPLISQL_DTYPE_VAR,PLISQL_DTYPE_PROMISE, andPLISQL_DTYPE_RECwhen computingfunction->copiable_size. IfPLiSQL_exception_vardatums are:
- Purely compile-time metadata (no per-execution mutable state), then excluding them here is fine.
- Expected to be duplicated by
copy_plisql_datums()for each execution, then this switch and the copy routine should stay in sync to avoid size mismatches.Please confirm that
copy_plisql_datums()either ignoresPLISQL_DTYPE_EXCEPTIONor handles it without relying oncopiable_size, and adjust this switch or add a comment if needed to keep the contracts aligned.
3309-3342: Initialization list deliberately skips exception datums—worth a brief note
plisql_add_initdatums()continues to treat only VAR and REC datums as “initializable” at block entry. For user-defined exceptions, that’s probably correct (they behave like named constants bound to SQLCODE), but it means they’ll never be automatically cleared or reinitialized per block entry.If that’s the intended semantics, a short comment in either this function or the new exception-type implementation clarifying that
PLISQL_DTYPE_EXCEPTIONcarries no per-block state would help future maintainers avoid accidentally adding it here.src/pl/plisql/src/sql/plisql_exception.sql (1)
9-113: Test coverage for declarations and basic raise/catch looks good; consider adding PRAGMA testsThe new tests cover:
- Package-level and local EXCEPTION declarations (including mixed with other variable types).
- A standalone procedure with a local EXCEPTION.
- Raising and catching a user-defined exception in
test_exc_raise.test_raise_catch.This aligns well with the new runtime behavior. Given that the PR also introduces
PRAGMA EXCEPTION_INIT, it would be valuable (though not strictly required for this PR) to add at least one test that:
- Declares an exception.
- Uses
PRAGMA EXCEPTION_INITto bind it to a specific SQLCODE.- Verifies that a RAISE of that SQLCODE is caught by
WHEN <exception_name>.That would fully exercise the new API from SQL and protect against regressions in the binding logic.
src/pl/plisql/src/pl_exec.c (2)
1510-1515: EnsurePLISQL_DTYPE_EXCEPTIONis treated as non-copiable consistentlyHandling
PLISQL_DTYPE_EXCEPTIONincopy_plisql_datumsby just aliasingoutdatum = indatum(like ROW and PACKAGE_DATUM) makes sense given exceptions are metadata-like and have no per-execution mutable state.However, the comment explicitly says this switch must agree with
plisql_finish_datumson what is copiable. Please double‑check thatplisql_finish_datums(in the compiler side) also classifiesPLISQL_DTYPE_EXCEPTIONas non‑copiable; otherwisefunc->copiable_sizecould be wrong and lead to buffer overruns or under-allocation.
4049-4050: RAISE support forexception_varis largely correct; minor text nit onlyThe changes to
exec_stmt_raisedo the right structural things:
RAISErethrow is now gated onstmt->exception_var == NULL, soRAISE custom_exc;is not mistaken for a bare rethrow.- When
stmt->exception_varis present, you seederr_codefromexc->sqlcodeandcondnamefromexc->refname, and only synthesize the default message"User-Defined Exception"when neitherMESSAGEnor other options are provided. That aligns with Oracle-styleRAISE my_exc;behavior.- Options like
ERRCODE,MESSAGE, etc. still override the defaults, and double‑specification of ERRCODE is correctly rejected.One minor, optional polish point: Oracle’s built-in text for this case is
"user-defined exception"(all lower case). If you care about byte-for-byte compatibility for client tests, you might want to mirror that exact string (and possibly run it through gettext).Otherwise this block looks solid.
Also applies to: 4062-4076
src/pl/plisql/src/pl_gram.y (1)
349-350: Token and keyword-list wiring forK_EXCEPTION_INIT/K_PRAGMAis mostly consistent; verifyunit_name_keywordcoverage forK_PRAGMA
- Adding
K_EXCEPTION_INITandK_PRAGMAas<keyword>tokens and wiringK_EXCEPTION_INITinto bothunreserved_keywordandunit_name_keywordis consistent with how other unreserved keywords are exposed to the grammar.K_PRAGMAis added tounreserved_keywordbut not tounit_name_keyword. This meansPRAGMAcannot be used whereunit_name_keywordis allowed as a unit name (e.g., inunit_name), unlike most other unreserved keywords.If the intent is that
PRAGMAshould not be usable as a function/procedure/unit name, the current setup is fine. If instead you wantPRAGMAto behave like other unreserved keywords, consider adding it tounit_name_keywordas well.No functional change is needed unless you specifically want
PRAGMAto be usable as a unit name.Also applies to: 395-396, 3281-3282, 3314-3315, 3390-3391
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/pl/plisql/src/expected/plisql_exception.outis excluded by!**/*.out
📒 Files selected for processing (11)
src/pl/plisql/src/Makefile(2 hunks)src/pl/plisql/src/pl_comp.c(3 hunks)src/pl/plisql/src/pl_exception_type.c(1 hunks)src/pl/plisql/src/pl_exception_type.h(1 hunks)src/pl/plisql/src/pl_exec.c(14 hunks)src/pl/plisql/src/pl_funcs.c(3 hunks)src/pl/plisql/src/pl_gram.y(9 hunks)src/pl/plisql/src/pl_package.c(1 hunks)src/pl/plisql/src/pl_unreserved_kwlist.h(2 hunks)src/pl/plisql/src/plisql.h(2 hunks)src/pl/plisql/src/sql/plisql_exception.sql(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:
src/pl/plisql/src/Makefile
🧬 Code graph analysis (5)
src/pl/plisql/src/pl_comp.c (1)
src/pl/plisql/src/pl_exception_type.c (1)
plisql_lookup_exception(82-96)
src/pl/plisql/src/pl_exception_type.c (4)
src/backend/utils/mmgr/mcxt.c (2)
palloc0(1350-1370)pstrdup(1702-1706)src/pl/plisql/src/pl_comp.c (1)
plisql_adddatum(3219-3230)src/pl/plisql/src/pl_funcs.c (3)
plisql_ns_additem(101-117)plisql_ns_lookup(140-198)plisql_ns_top(90-94)src/pl/plisql/src/pl_scanner.c (1)
plisql_scanner_errposition(613-627)
src/pl/plisql/src/pl_funcs.c (1)
src/pl/plisql/src/pl_exception_type.c (1)
plisql_dump_exception(155-163)
src/pl/plisql/src/pl_exception_type.h (1)
src/pl/plisql/src/pl_exception_type.c (7)
plisql_build_exception(29-50)plisql_datum_is_exception(56-63)plisql_datum_get_exception(69-76)plisql_lookup_exception(82-96)plisql_exception_set_sqlcode(105-112)plisql_process_pragma_exception_init(123-149)plisql_dump_exception(155-163)
src/pl/plisql/src/pl_exec.c (1)
src/backend/utils/mmgr/mcxt.c (1)
MemoryContextStrdup(1689-1700)
🔇 Additional comments (17)
src/pl/plisql/src/pl_exception_type.c (2)
82-96: Namespace lookup and PRAGMA handler behavior look consistentThe combination of
plisql_lookup_exceptionandplisql_process_pragma_exception_initcorrectly:
- Resolves names through the current namespace using existing
plisql_ns_lookuprules.- Verifies that the resolved datum is actually an exception (
dtype == PLISQL_DTYPE_EXCEPTION) before proceeding.- Reports meaningful errors with
ERRCODE_UNDEFINED_OBJECTandERRCODE_WRONG_OBJECT_TYPE, with locations viaplisql_scanner_errposition.This matches how other PL/iSQL name-resolution paths behave and should give users clear feedback when PRAGMA EXCEPTION_INIT references a missing or non-exception name.
Also applies to: 123-149
52-76: Helper functions are minimal and type-safe
plisql_datum_is_exception,plisql_datum_get_exception,plisql_exception_set_sqlcode, andplisql_dump_exceptionare straightforward, null‑safe, and respect the newPLISQL_DTYPE_EXCEPTIONdiscriminator. Using a dedicated getter plus the dumper integration will keep future changes to exception representation localized.Also applies to: 99-112, 151-162
src/pl/plisql/src/pl_package.c (1)
3022-3085: Explicitly rejecting exceptions inis_const_datumis reasonableAdding an explicit
PLISQL_DTYPE_EXCEPTIONarm that raises"exception variables cannot be used in this context"makes the package-status logic fail fast if an exception datum ever leaks intois_const_datum. That’s preferable to silently treating it as const/non-const and matches the way this file handles other forbidden constructs (e.g., disallowed cursor vars in packages).No further changes needed here.
src/pl/plisql/src/Makefile (1)
40-52: Build and regression wiring for exceptions looks correctAdding
pl_exception_type.otoOBJSandplisql_exceptiontoREGRESScleanly integrates the new exception module into the PL/iSQL shared library and test suite. This aligns with how other PL/iSQL features are built and tested; nothing further needed in this Makefile.Also applies to: 57-62
src/pl/plisql/src/pl_funcs.c (1)
23-23: Exception datums are correctly integrated into free/dump paths
- Including
pl_exception_type.hhere is the right place to pick upPLiSQL_exception_varandplisql_dump_exceptionwithout polluting public headers.- In
plisql_free_function_memory, the addedPLISQL_DTYPE_EXCEPTIONarm that justbreaks is appropriate: exception datums don’t own SPI plans or other resources, and the enclosing memory context free will reclaim their storage.- In
plisql_dumptree, delegating the newPLISQL_DTYPE_EXCEPTIONcase toplisql_dump_exceptionmatches the existing debug-printing style and will make exception variables visible when inspecting compiled trees.These changes look consistent and low‑risk.
Also applies to: 741-771, 1677-1760
src/pl/plisql/src/plisql.h (1)
898-909: ExtendingPLiSQL_stmt_raisewithexception_varis a clean hookThe added
struct PLiSQL_exception_var *exception_varfield onPLiSQL_stmt_raisegives the executor a direct link to the user-defined exception datum without changing existing RAISE semantics. Using an opaque struct pointer avoids exposing the full exception layout in this public header and keeps the detailed definition inpl_exception_type.h.Assuming the parser always initializes this to NULL for non-exception RAISE forms and sets it only for
RAISE <exception_var>cases, this is a straightforward and backward-compatible extension.src/pl/plisql/src/pl_unreserved_kwlist.h (2)
69-71: Newexception_initkeyword is correctly integrated
exception_initis placed afterexceptionand respects the ASCII sort requirement for this list; name matches the grammar intent forPRAGMA EXCEPTION_INIT.
103-106:pragmaunreserved keyword addition looks correct
pragmais positioned betweenpipelinedandprint_strict_params, consistent with the sorted keyword list and the intended use in PL/iSQL PRAGMA syntax.src/pl/plisql/src/pl_comp.c (2)
18-33: Includingpl_exception_type.hhere is appropriateThe additional include cleanly exposes
PLiSQL_exception_varandplisql_lookup_exception()to this compilation unit and matches how other PL/iSQL helper headers are wired.
3148-3165: User-defined exceptions are now correctly recognized inWHENconditionsThe new pre-check via
plisql_lookup_exception(condname)integrates package/localEXCEPTIONdeclarations intoplisql_parse_err_condition, soEXCEPTION WHEN custom_error THENbinds to the user-defined exception’ssqlcodebefore consultingOTHERSor the static label map. This matches the PR goal of Oracle-compatible user-defined exceptions.Two small points to double-check:
- Name shadowing: if a user-defined exception uses the same name as a built-in condition label, this code now prefers the user-defined one. If that’s intentional, it would be good to rely on tests to lock this in; if not, you might instead error on such redefinitions.
- Namespace timing:
plisql_lookup_exception()uses the current namespace; ensure the grammar only callsplisql_parse_err_conditionafter exception declarations for the block/package have been registered, which seems to be the case but is worth keeping in mind for future grammar edits.Overall the integration looks sound and aligns with the new test
test_exc_raise.src/pl/plisql/src/pl_exec.c (4)
45-45: Include ofpl_exception_type.his appropriateIncluding the new exception-type header here is consistent with how other PL/iSQL types are wired into the executor. No issues.
919-925: Rejecting exception vars as anonymous-block OUT parameters is correctRaising an ERROR when encountering
PLISQL_DTYPE_EXCEPTIONin the anonymous-block out-parameter marshalling path matches the intent that exception variables are not value-carrying and must not appear in OUT slots. This is a good defensive check.
1915-1920: Skipping block-time init for exception vars is fineTreating
PLISQL_DTYPE_EXCEPTIONas “nothing to do” during block local-variable initialization is consistent with them being declaration-only symbols with no runtime storage. This looks correct.
9251-9257: Good: assignability checks explicitly forbid writes to exception varsBoth
exec_check_packagedatum_assignableandexec_check_assignablenow treatPLISQL_DTYPE_EXCEPTIONas an error with the standard message"exception variables cannot be used in this context". That’s exactly what you want to gate exception vars to declaration/RAISE/WHEN usage only and ensures assignment attempts are rejected early and consistently.Also applies to: 9330-9336
src/pl/plisql/src/pl_gram.y (2)
34-34: Includingpl_exception_type.hhere is appropriateThe parser now legitimately depends on
PLiSQL_exception_varand the related helpers; includingpl_exception_type.halongside the other PL/iSQL headers is consistent with existing layering.
2493-2493: RAISE handling for user-defined exceptions looks correct, with one nuance to be aware of
- Initializing
new->exception_var = NULL;alongside the otherPLiSQL_stmt_raisefields avoids uninitialized reads and is consistent with how other optional attributes are handled.- The new branch:
correctly:if (tok == T_DATUM && plisql_datum_is_exception(yylval.wdatum.datum)) new->exception_var = (PLiSQL_exception_var *) yylval.wdatum.datum; else if (tok == T_WORD) ...
- Detects a datum that is an exception variable (including package-level exceptions, since they’re still datums) and records it in
exception_var.- Falls back to the existing condition-name resolution path for non-exception tokens and validates those via
plisql_recognize_err_conditiononly whenexception_varis NULL.Nuances:
- For
RAISE my_exc;andRAISE EXCEPTION my_exc;you will now emit the exception frommy_excwithout trying to resolve a condition name, which matches the intended design.- For a
T_DATUMthat isn’t an exception, the grammar still errors out via the existingyyerror("syntax error")path before callingplisql_recognize_err_condition, socondnameis never dereferenced when NULL.No changes needed here.
If you haven’t already, please add regression cases covering:
RAISE my_exc;RAISE EXCEPTION my_exc;RAISE my_pkg.my_exc;(package-level exception)
with and withoutUSINGoptions, to ensure executor behavior matches Oracle expectations.Also applies to: 2600-2616
src/pl/plisql/src/pl_exception_type.h (1)
68-93: Public exception API surface looks coherentThe set of exported helpers (
plisql_build_exception,plisql_datum_is_exception,plisql_datum_get_exception,plisql_lookup_exception,plisql_exception_set_sqlcode,plisql_process_pragma_exception_init,plisql_dump_exception) matches the usage patterns in the grammar and executor snippets:
- Builder used from DECLARE section for
name EXCEPTION;.- Lookup and
*_is_exceptionused by RAISE/handler code paths.plisql_process_pragma_exception_initcleanly separates PRAGMA semantics from the grammar.plisql_dump_exceptionfits the existing debug/plisql_dump_*pattern.No structural issues here.
| PLiSQL_exception_var * | ||
| plisql_build_exception(const char *refname, int lineno, bool add2namespace) | ||
| { | ||
| PLiSQL_exception_var *exc; | ||
|
|
||
| exc = palloc0(sizeof(PLiSQL_exception_var)); | ||
| exc->dtype = PLISQL_DTYPE_EXCEPTION; | ||
| exc->refname = pstrdup(refname); | ||
| exc->lineno = lineno; | ||
| exc->sqlcode = ERRCODE_RAISE_EXCEPTION; /* Default P0001 for user-defined exceptions */ | ||
| exc->pkgoid = InvalidOid; | ||
|
|
||
| /* Assign datum number and add to datum array */ | ||
| exc->dno = plisql_nDatums; | ||
| plisql_adddatum((PLiSQL_datum *) exc); | ||
|
|
||
| /* Add to namespace if requested */ | ||
| if (add2namespace) | ||
| plisql_ns_additem(PLISQL_NSTYPE_VAR, exc->dno, refname); | ||
|
|
||
| return exc; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify sqlcode semantics and clean up dno assignment
Two points here:
-
exc->sqlcodeis initialized toERRCODE_RAISE_EXCEPTIONbut the field is namedsqlcodeand the PR description says “default SQLCODE is 1” for user-defined exceptions. If this field is meant to hold an Oracle-style SQLCODE (e.g. 1, -20001, …), using a SQLSTATE macro here is confusing and likely incorrect; if it’s really a SQLSTATE, consider renaming the field (and adjusting PRAGMA EXCEPTION_INIT to pass a SQLSTATE-like value rather than an Oracle code). Please double‑check intended usage across pl_exec.c / error mapping. -
exc->dno = plisql_nDatums;is immediately overwritten insideplisql_adddatum(), which assigns the datum number and incrementsplisql_nDatums. The current comment (“Assign datum number and add to datum array”) is misleading. You can safely drop the manual assignment and rely onplisql_adddatum:
- /* Assign datum number and add to datum array */
- exc->dno = plisql_nDatums;
- plisql_adddatum((PLiSQL_datum *) exc);
+ /* Add to datum array; plisql_adddatum assigns dno */
+ plisql_adddatum((PLiSQL_datum *) exc);
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for plisql_adddatum implementation and how it handles dno
rg -n "plisql_adddatum" -A 8 -B 2 --type=c
Length of output: 9275
🏁 Script executed:
#!/bin/bash
# Search for ERRCODE_RAISE_EXCEPTION definition
rg -n "ERRCODE_RAISE_EXCEPTION" --type=cLength of output: 273
🏁 Script executed:
#!/bin/bash
# Search for sqlcode usage in pl_exec.c and error handling
rg -n "sqlcode" --type=c -C 3Length of output: 292503
🏁 Script executed:
#!/bin/bash
# Search for PRAGMA EXCEPTION_INIT handler
rg -n "EXCEPTION_INIT|PRAGMA.*EXCEPTION" --type=c -C 4Length of output: 4717
🏁 Script executed:
#!/bin/bash
# Search for the exception_var struct definition to understand field types
rg -n "PLiSQL_exception_var|sqlcode" --type=h -C 5Length of output: 6737
Clarify sqlcode semantics (SQLSTATE vs Oracle SQLCODE) and remove redundant dno assignment
-
SQLCODE semantics: confirmed that exc->sqlcode is treated as a SQLSTATE/SQLERRSTATE at runtime (pl_exec.c uses it as err_code; pl_comp.c assigns it into new->sqlerrstate), but the header/docs still describe an Oracle-style SQLCODE/default of 1 and PRAGMA EXCEPTION_INIT accepts an int. Make the intended semantics explicit: either
- treat the field as a SQLSTATE (rename sqlcode → sqlerrstate, update comments and docs, and require PRAGMA EXCEPTION_INIT to accept a SQLSTATE or a SQLSTATE-like value), or
- keep Oracle-style numeric SQLCODEs but implement a deterministic mapping/conversion at compile time when PRAGMA EXCEPTION_INIT is processed, and document that mapping.
Files to update: src/pl/plisql/src/pl_exception_type.h, src/pl/plisql/src/pl_exception_type.c, src/pl/plisql/src/pl_exec.c, and the PRAGMA handler in src/pl/plisql/src/pl_exception_type.c.
-
Redundant dno assignment: plisql_adddatum() assigns the datum number; remove the manual assignment in the builder. Suggested change:
- /* Assign datum number and add to datum array */
- exc->dno = plisql_nDatums;
- plisql_adddatum((PLiSQL_datum *) exc);
+ /* Add to datum array; plisql_adddatum assigns dno */
+ plisql_adddatum((PLiSQL_datum *) exc);| /* | ||
| * Exception datum type enum value | ||
| * NOTE: This must be added to PLiSQL_datum_type enum in plisql.h: | ||
| * PLISQL_DTYPE_EXCEPTION = 7 | ||
| */ | ||
| #define PLISQL_DTYPE_EXCEPTION 7 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Keep dtype constant definition in sync with the core enum and avoid confusing SQLCODE vs SQLSTATE semantics
Two related points here:
-
Brittle duplication of the dtype value
#define PLISQL_DTYPE_EXCEPTION 7is manually duplicated with a comment saying this must match thePLiSQL_datum_typeenum inplisql.h.- This is easy to desync if someone later inserts new DTYPEs.
Prefer to define
PLISQL_DTYPE_EXCEPTIONonly once in thePLiSQL_datum_typeenum and include that header here, rather than also defining a hardcoded macro. If you need the constant here, rely on the enum symbol, not a separate#define. -
Header comment vs implementation for
sqlcodedefault- The header says: “Default is 1 for user-defined exceptions.”
- The implementation in
plisql_build_exceptioninitializesexc->sqlcode = ERRCODE_RAISE_EXCEPTION; /* Default P0001 for user-defined exceptions */, which is a SQLSTATE-style Postgres error code, not Oracle’sSQLCODE = 1.
It would be good to clarify the intended semantics and make the comment consistent with the implementation, e.g., explicitly state that
sqlcodeholds the internal error code/SQLSTATE used when raising, and that the Oracle-visibleSQLCODE/SQLERRMmapping happens elsewhere.
Suggested tweak to the comment:
- * sqlcode is the error code associated with this exception.
- * Default is 1 for user-defined exceptions.
- * Can be set to a specific Oracle error code via PRAGMA EXCEPTION_INIT.
+ * sqlcode is the internal error code associated with this exception.
+ * By default this is ERRCODE_RAISE_EXCEPTION (SQLSTATE P0001) for
+ * user-defined exceptions, and it can be remapped via PRAGMA
+ * EXCEPTION_INIT to correspond to a specific Oracle error number.Please double-check that the intended mapping between Oracle SQLCODE values and this sqlcode field is correctly implemented in the executor (e.g., where the exception is actually raised) and that tests cover both the default and PRAGMA EXCEPTION_INIT paths.
Also applies to: 48-62
I'll verify the review comment by examining the codebase. Let me search for the enum definition, implementation details, and actual comments.
🏁 Script executed:
#!/bin/bash
# 1. Find PLiSQL_datum_type enum in plisql.h
echo "=== Searching for PLiSQL_datum_type enum ==="
rg -n "enum\s+PLiSQL_datum_type" --type=c --type=h
echo ""
echo "=== Enum values around EXCEPTION ==="
rg -n "PLISQL_DTYPE" --type=c --type=h -A2 -B2 | head -50
echo ""
echo "=== Content of pl_exception_type.h around lines 33-62 ==="
fd "pl_exception_type.h" --type f --exec cat {} \;
echo ""
echo "=== Searching for plisql_build_exception implementation ==="
rg -n "plisql_build_exception" --type=c --type=h -A10 -B2 | head -80Length of output: 7997
Now let me check for test coverage and verify the includes:
🏁 Script executed:
#!/bin/bash
# Check includes in pl_exception_type.h
echo "=== Includes in pl_exception_type.h ==="
head -n 40 src/pl/plisql/src/pl_exception_type.h | grep -E "^#include"
echo ""
echo "=== Verify enum value for PLISQL_DTYPE_EXCEPTION ==="
# Count enum members to confirm EXCEPTION is indeed position 7 (0-indexed = 6)
rg -A 10 "typedef enum PLiSQL_datum_type" src/pl/plisql/src/plisql.h
echo ""
echo "=== Search for exception tests ==="
fd -e c -e sql -e pl -e test | xargs grep -l "exception\|EXCEPTION" 2>/dev/null | grep -i test | head -20
echo ""
echo "=== Look for PRAGMA EXCEPTION_INIT references ==="
rg "PRAGMA_EXCEPTION_INIT|EXCEPTION_INIT" --type=c --type=h -n | head -20Length of output: 2112
Fix brittle dtype macro duplication and resolve comment/implementation mismatch; verify correct enum value
The review comment's two concerns are confirmed:
-
Duplicate macro confirmed: The
#define PLISQL_DTYPE_EXCEPTION 7at line 35 duplicates the enum constant already present inplisql.h:73. Sinceplisql.his included here, relying on the enum symbol directly eliminates maintenance risk if new dtype members are inserted. -
Comment vs. implementation mismatch confirmed: Header comment (lines 57–59) states "Default is 1 for user-defined exceptions," but the implementation in
pl_exception_type.c:38initializesexc->sqlcode = ERRCODE_RAISE_EXCEPTION; /* Default P0001 */. These are different semantics (Oracle-style numeric code vs. PostgreSQL SQLSTATE). -
Critical: Enum value mismatch: The enum
PLiSQL_datum_typeinplisql.hhas 7 members (VAR, ROW, REC, RECFIELD, PROMISE, PACKAGE_DATUM, EXCEPTION), making EXCEPTION the 6th member (0-indexed value 6). However, the macro is hardcoded as7. Verify this is intentional or correct the macro value to6.
Recommended fixes:
- Remove the
#defineand use the enum symbolPLISQL_DTYPE_EXCEPTIONdirectly. - Update the header comment to clarify: "Default is ERRCODE_RAISE_EXCEPTION (SQLSTATE P0001)" and explain that Oracle-visible SQLCODE mapping occurs elsewhere.
- Verify and confirm the enum value is indeed 6, not 7.
| case PLISQL_DTYPE_EXCEPTION: | ||
| /* | ||
| * Exception variables - nothing to do | ||
| */ | ||
| break; | ||
|
|
There was a problem hiding this comment.
RETURN / RETURN NEXT on exception variables should ERROR, not silently no-op
Both switches treat PLISQL_DTYPE_EXCEPTION as a no-op:
- In
exec_stmt_return, thePLISQL_DTYPE_EXCEPTIONarm does nothing, soestate->retval,retisnull, andrettyperemain whatever they were initialized to for the statement (typically “NULL with InvalidOid type”). In domain-returning functions that can later force a cast on NULL, this combination is dubious, and for non-domain returns it quietly turnsRETURN exc_var;into “return NULL”. - In
exec_stmt_return_next, thePLISQL_DTYPE_EXCEPTIONarm is also a no-op, soRETURN NEXT exc_var;silently produces no row.
Everywhere else you’re explicitly rejecting misuse of exception variables with "exception variables cannot be used in this context", so silently accepting these constructs is inconsistent and likely to hide bugs.
I recommend making these cases explicit errors, aligned with other sites:
@@ case PLISQL_DTYPE_RECFIELD:
- case PLISQL_DTYPE_EXCEPTION:
- /*
- * Exception variables - nothing to do
- */
- break;
+ case PLISQL_DTYPE_EXCEPTION:
+ /* Exception variables are not valid RETURN targets */
+ elog(ERROR, "exception variables cannot be used in this context");
+ break;and similarly in exec_stmt_return_next:
@@ switch (retvar->dtype)
- case PLISQL_DTYPE_EXCEPTION:
- /*
- * Exception variables - nothing to do
- */
- break;
+ case PLISQL_DTYPE_EXCEPTION:
+ /* Exception variables are not valid in RETURN NEXT */
+ elog(ERROR, "exception variables cannot be used in this context");
+ break;This keeps semantics clear and avoids propagating incomplete return metadata.
Also applies to: 3738-3743
🤖 Prompt for AI Agents
In src/pl/plisql/src/pl_exec.c around lines 3555-3560 (exec_stmt_return) and
3738-3743 (exec_stmt_return_next), the switch arms for PLISQL_DTYPE_EXCEPTION
are currently no-ops; change them to explicitly throw an error instead of
silently doing nothing. Replace the empty cases with the same error handling
used elsewhere for exception-variable misuse (e.g. the "exception variables
cannot be used in this context" diagnostic), using the appropriate ereport/ERROR
path and message tailored to the statement ("exception variables cannot be used
in RETURN" for exec_stmt_return and "exception variables cannot be used in
RETURN NEXT" for exec_stmt_return_next) so callers get a clear error rather than
producing NULL/omitted rows or invalid return metadata.
| case PLISQL_DTYPE_EXCEPTION: | ||
| /* | ||
| * Exception variables - nothing to do | ||
| */ | ||
| break; | ||
|
|
There was a problem hiding this comment.
Generic datum/typing paths should reject PLISQL_DTYPE_EXCEPTION instead of falling through
Several core helpers now have a PLISQL_DTYPE_EXCEPTION arm that just comments “nothing to do” and breaks (or returns), without setting output fields:
exec_assign_value(lines 5646‑5651)exec_eval_datum(lines 5833‑5838)plisql_exec_get_datum_type(lines 5933‑5938)plisql_exec_get_datum_type_info(lines 6049‑6053)
Today the grammar very likely prevents exception variables from ever reaching these generic paths, but if that assumption is ever broken (e.g., future refactor or a missed semantic check), these branches will:
- Leave
*typeid,*typetypmod,*value,*isnulluninitialized inexec_eval_datum/ type getters, leading to undefined behavior or obscure crashes. - Turn assignments to exception variables into silent no-ops if some caller doesn’t go through
exec_check_assignablefirst.
Given you already centralize the policy in exec_check_assignable / exec_check_packagedatum_assignable and in the anonymous-outparam path by raising:
elog(ERROR, "exception variables cannot be used in this context");it would be safer and more consistent to do the same here, rather than “do nothing”.
Concretely, for each of these switches, I’d suggest:
- case PLISQL_DTYPE_EXCEPTION:
- /*
- * Exception variables - nothing to do
- */
- break;
+ case PLISQL_DTYPE_EXCEPTION:
+ elog(ERROR, "exception variables cannot be used in this context");
+ break;and similarly in the two type-query helpers. That keeps all generic datum paths robust even if a future change lets an exception datum leak through.
Also applies to: 5833-5838, 5933-5938, 6049-6053
🤖 Prompt for AI Agents
In src/pl/plisql/src/pl_exec.c around lines 5646-5651 (and similarly at
5833-5838, 5933-5938, 6049-6053), the switch arms for PLISQL_DTYPE_EXCEPTION
currently do nothing and return/break, leaving output fields uninitialized or
silently ignoring assignments; change each of those arms to raise an error
(elog(ERROR, "exception variables cannot be used in this context");) so the
generic datum/typing helpers reject exception variables instead of falling
through or leaving results undefined.
| | decl_varname K_EXCEPTION ';' | ||
| { | ||
| PLiSQL_exception_var *exc; | ||
|
|
||
| exc = plisql_build_exception($1.name, $1.lineno, true); | ||
| } | ||
| | K_PRAGMA K_EXCEPTION_INIT '(' decl_varname ',' ICONST ')' ';' | ||
| { | ||
| plisql_process_pragma_exception_init($4.name, | ||
| $6, | ||
| @1, yyscanner); | ||
| } |
There was a problem hiding this comment.
PRAGMA EXCEPTION_INIT grammar incorrectly reuses decl_varname (blocks valid usage) and leaves an unused local
Two issues in this decl arm:
-
Semantic bug (blocks normal Oracle-style usage)
K_PRAGMA K_EXCEPTION_INIT '(' decl_varname ',' ICONST ')' ';'usesdecl_varname.decl_varnameenforces “name isn't already declared in the current block” and raises"duplicate declaration"if it finds one.- But
plisql_process_pragma_exception_initthen looks up an already-declared exception by name and errors if it does not exist. - Combined, this makes the intended pattern:
impossible: the second line will hit the duplicate-declaration check before your PRAGMA helper ever runs.
my_exc EXCEPTION; PRAGMA EXCEPTION_INIT(my_exc, -20000);
You want the PRAGMA to reference an existing identifier, not declare a new one. This arm should accept a plain identifier (e.g.
any_identifier) without duplicate-declaration checks. -
Unused local variable
exc- In
decl_varname K_EXCEPTION ';'the localPLiSQL_exception_var *exc;is never used after assignment, which will generate a compiler warning.
- In
Suggested fix
Replace decl_varname with any_identifier in the PRAGMA production and remove the unused local in the EXCEPTION declaration, e.g.:
- | decl_varname K_EXCEPTION ';'
- {
- PLiSQL_exception_var *exc;
-
- exc = plisql_build_exception($1.name, $1.lineno, true);
- }
- | K_PRAGMA K_EXCEPTION_INIT '(' decl_varname ',' ICONST ')' ';'
- {
- plisql_process_pragma_exception_init($4.name,
- $6,
- @1, yyscanner);
- }
+ | decl_varname K_EXCEPTION ';'
+ {
+ /* declare a user-defined exception variable */
+ plisql_build_exception($1.name, $1.lineno, true);
+ }
+ | K_PRAGMA K_EXCEPTION_INIT '(' any_identifier ',' ICONST ')' ';'
+ {
+ /* bind an existing exception variable to a sqlcode */
+ plisql_process_pragma_exception_init($4,
+ $6,
+ @4, yyscanner);
+ }This allows the canonical pattern “exc EXCEPTION; PRAGMA EXCEPTION_INIT(exc, code);” and eliminates the unused local.
| -- | ||
| -- Test user-defined EXCEPTION type declarations | ||
| -- | ||
| -- Note: This test verifies EXCEPTION type declarations compile correctly. | ||
| -- RAISE and WHEN handler support for user-defined exceptions is not yet | ||
| -- implemented and is commented out below. | ||
| -- |
There was a problem hiding this comment.
Top-of-file comment is now stale relative to the tests
The header still says that RAISE and WHEN handler support for user-defined exceptions is “not yet implemented” and “commented out below”, but Test 5 now actively raises and catches custom_error.
Recommend updating this comment to reflect the current scope, e.g.:
---
--- Test user-defined EXCEPTION type declarations
---
--- Note: This test verifies EXCEPTION type declarations compile correctly.
--- RAISE and WHEN handler support for user-defined exceptions is not yet
--- implemented and is commented out below.
---
+--
+-- Tests for user-defined EXCEPTION declarations and basic RAISE/WHEN handling.
+--📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- | |
| -- Test user-defined EXCEPTION type declarations | |
| -- | |
| -- Note: This test verifies EXCEPTION type declarations compile correctly. | |
| -- RAISE and WHEN handler support for user-defined exceptions is not yet | |
| -- implemented and is commented out below. | |
| -- | |
| -- | |
| -- Tests for user-defined EXCEPTION declarations and basic RAISE/WHEN handling. | |
| -- |
🤖 Prompt for AI Agents
In src/pl/plisql/src/sql/plisql_exception.sql around lines 1 to 7, the
top-of-file comment is stale: it claims RAISE and WHEN handler support for
user-defined exceptions is "not yet implemented" and "commented out below", but
Test 5 now raises and catches custom_error. Update the header comment to
accurately describe current test behavior and supported features—remove or
rephrase the note about missing RAISE/WHEN support, and mention that
user-defined EXCEPTIONs can be raised and handled (or reflect the exact current
test coverage) so the comment matches the file contents.
Thanks! |
|
Thank you for your new contribution. |
|
|
||
| exc = plisql_build_exception($1.name, $1.lineno, true); | ||
| } | ||
| | K_PRAGMA K_EXCEPTION_INIT '(' decl_varname ',' ICONST ')' ';' |
There was a problem hiding this comment.
Given that I haven't seen any test content about EXCEPTION_INIT in the regression testing, I attempted to execute the following tests:
--print handle zero excep
DECLARE
zero_excep exception;
PRAGMA exception_init(zero_excep, -1476);
var1 integer;
BEGIN
var1 := 100 / 0;
exception
WHEN zero_excep THEN
raise info 'handle zero excep';
end;
/
Output syntax error...
Hmm, I'm guessing this part hasn't been implemented yet.
Perhaps we could add two new types, such as any_identifierand exception_error_code.
| K_PRAGMA K_EXCEPTION_INIT '(' any_identifier ',' exception_error_code ')' ';'
There was a problem hiding this comment.
Thanks, noted in rophy#5. I'll try to take care of this over upcoming weekend.
There was a problem hiding this comment.
Highly efficient output and the test cases are comprehensive and helpful.
I don't have any new review comments.
Thanks.
There was a problem hiding this comment.
Since we're adding user defined error codes, I spent some time testing the valid range for the error codes.
https://docs.oracle.com/en/database/oracle/oracle-database/19/lnpls/EXCEPTION_INIT-pragma.html
Error code to be associated with exception. error_code can be either 100 (the numeric code for "no data found" that "SQLCODE Function" returns) or any negative integer greater than -1000000 except -1403 (another numeric code for "no data found").
Added commit 1622236 to get consistent behavior as oracle - tested with "oracle database free" container 23.26.0.0.-lite.
There was a problem hiding this comment.
Oracle PRAGMA EXCEPTION_INIT Validation Test Report
Test Date: 2025-11-27
Oracle Version: Oracle Database 23ai Free (23.26.0.0-lite)
Purpose: Validate Oracle behavior for PRAGMA EXCEPTION_INIT error codes
Summary
| Test | Error Code | Expected | Oracle Result | Status |
|---|---|---|---|---|
| 10a | 100 |
SUCCESS | SUCCESS | ✅ PASS |
| 10b | 1 |
PLS-00701 | PLS-00701 | ✅ PASS |
| 10c | 1000000 |
PLS-00701 | PLS-00701 | ✅ PASS |
| 11a | -1000000 |
SUCCESS | SUCCESS | ✅ PASS |
| 11b | -1000001 |
PLS-00701 | PLS-00701 | ✅ PASS |
| 11c | -1403 |
PLS-00701 | PLS-00701 | ✅ PASS |
| 11d | 0 |
PLS-00701 | PLS-00701 | ✅ PASS |
| Extra | -20001 |
SUCCESS | SUCCESS | ✅ PASS |
All 8 tests passed. IvorySQL validation logic matches Oracle behavior.
Detailed Test Results
Test 10a: Valid positive error code 100
Input:
CREATE OR REPLACE PACKAGE test_pragma_positive_100 IS
PROCEDURE test_positive;
END test_pragma_positive_100;
/
CREATE OR REPLACE PACKAGE BODY test_pragma_positive_100 IS
exc_100 EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_100, 100);
PROCEDURE test_positive IS
BEGIN
DBMS_OUTPUT.PUT_LINE('Error code 100 accepted (ANSI NO_DATA_FOUND)');
END test_positive;
END test_pragma_positive_100;
/
BEGIN
test_pragma_positive_100.test_positive();
END;
/Output:
Package created.
Package body created.
Error code 100 accepted (ANSI NO_DATA_FOUND)
PL/SQL procedure successfully completed.
Result: ✅ SUCCESS - Error code 100 (ANSI NO_DATA_FOUND) is accepted.
Test 10b: Invalid positive error code 1
Input:
CREATE OR REPLACE PACKAGE test_pragma_positive_1 IS
exc_1 EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_1, 1);
END test_pragma_positive_1;
/Output:
Warning: Package created with compilation errors.
Errors for PACKAGE TEST_PRAGMA_POSITIVE_1:
LINE/COL ERROR
-------- -----------------------------------------------------------------
2/3 PLS-00701: illegal ORACLE error number 1 for PRAGMA
EXCEPTION_INIT
Result: ✅ REJECTED - Positive codes other than 100 are rejected with PLS-00701.
Test 10c: Invalid positive error code 1000000
Input:
CREATE OR REPLACE PACKAGE test_pragma_positive_1000000 IS
exc_1000000 EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_1000000, 1000000);
END test_pragma_positive_1000000;
/Output:
Warning: Package created with compilation errors.
Errors for PACKAGE TEST_PRAGMA_POSITIVE_1000000:
LINE/COL ERROR
-------- -----------------------------------------------------------------
2/3 PLS-00701: illegal ORACLE error number 1000000 for PRAGMA
EXCEPTION_INIT
Result: ✅ REJECTED - Large positive codes are rejected with PLS-00701.
Test 11a: Valid error code -1000000 (at boundary)
Input:
CREATE OR REPLACE PACKAGE test_pragma_minus_1000000 IS
PROCEDURE test_boundary;
END test_pragma_minus_1000000;
/
CREATE OR REPLACE PACKAGE BODY test_pragma_minus_1000000 IS
exc_boundary EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_boundary, -1000000);
PROCEDURE test_boundary IS
BEGIN
DBMS_OUTPUT.PUT_LINE('Error code -1000000 accepted (at boundary)');
END test_boundary;
END test_pragma_minus_1000000;
/
BEGIN
test_pragma_minus_1000000.test_boundary();
END;
/Output:
Package created.
Package body created.
Error code -1000000 accepted (at boundary)
PL/SQL procedure successfully completed.
Result: ✅ SUCCESS - Error code -1000000 (at boundary) is accepted.
Test 11b: Invalid error code -1000001 (beyond boundary)
Input:
CREATE OR REPLACE PACKAGE test_pragma_minus_1000001 IS
exc_beyond EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_beyond, -1000001);
END test_pragma_minus_1000001;
/Output:
Warning: Package created with compilation errors.
Errors for PACKAGE TEST_PRAGMA_MINUS_1000001:
LINE/COL ERROR
-------- -----------------------------------------------------------------
2/3 PLS-00701: illegal ORACLE error number -1000001 for PRAGMA
EXCEPTION_INIT
Result: ✅ REJECTED - Codes beyond -1000000 boundary are rejected with PLS-00701.
Test 11c: Invalid error code -1403
Input:
CREATE OR REPLACE PACKAGE test_pragma_minus_1403 IS
exc_1403 EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_1403, -1403);
END test_pragma_minus_1403;
/Output:
Warning: Package created with compilation errors.
Errors for PACKAGE TEST_PRAGMA_MINUS_1403:
LINE/COL ERROR
-------- -----------------------------------------------------------------
2/3 PLS-00701: illegal ORACLE error number -1403 for PRAGMA
EXCEPTION_INIT
Result: ✅ REJECTED - Error code -1403 (Oracle-specific NO_DATA_FOUND) is rejected; use 100 instead.
Test 11d: Invalid error code 0
Input:
CREATE OR REPLACE PACKAGE test_pragma_zero IS
exc_zero EXCEPTION;
PRAGMA EXCEPTION_INIT(exc_zero, 0);
END test_pragma_zero;
/Output:
Warning: Package created with compilation errors.
Errors for PACKAGE TEST_PRAGMA_ZERO:
LINE/COL ERROR
-------- -----------------------------------------------------------------
2/3 PLS-00701: illegal ORACLE error number 0 for PRAGMA
EXCEPTION_INIT
Result: ✅ REJECTED - Zero is not a valid error code.
Test Extra: Valid negative error code -20001
Input:
CREATE OR REPLACE PACKAGE test_pragma_minus_20001 IS
PROCEDURE test_user_defined;
END test_pragma_minus_20001;
/
CREATE OR REPLACE PACKAGE BODY test_pragma_minus_20001 IS
custom_exc EXCEPTION;
PRAGMA EXCEPTION_INIT(custom_exc, -20001);
PROCEDURE test_user_defined IS
BEGIN
DBMS_OUTPUT.PUT_LINE('Error code -20001 accepted (user-defined range)');
END test_user_defined;
END test_pragma_minus_20001;
/
BEGIN
test_pragma_minus_20001.test_user_defined();
END;
/Output:
Package created.
Package body created.
Error code -20001 accepted (user-defined range)
PL/SQL procedure successfully completed.
Result: ✅ SUCCESS - User-defined error codes (-20000 to -20999) are accepted.
Validation Rules Confirmed
Per Oracle documentation and testing, valid PRAGMA EXCEPTION_INIT error codes are:
- 100 - ANSI standard NO_DATA_FOUND
- Negative integers from -1 to -1000000 (inclusive), except:
- -1403 is rejected (Oracle-specific NO_DATA_FOUND; must use 100)
- 0 is rejected (not a valid error code)
Invalid codes receive error PLS-00701: illegal ORACLE error number N for PRAGMA EXCEPTION_INIT at compile time.
…ions Changed grammar from decl_varname to any_identifier to allow referencing already-declared exception variables. Added tests. Fixes #5
| * NOTE: This must be added to PLiSQL_datum_type enum in plisql.h: | ||
| * PLISQL_DTYPE_EXCEPTION = 7 | ||
| */ | ||
| #define PLISQL_DTYPE_EXCEPTION 7 |
There was a problem hiding this comment.
| #define PLISQL_DTYPE_EXCEPTION 7 |
Since PLISQL_DTYPE_EXCEPTION define in file plisql.h, we can remove it
typedef enum PLiSQL_datum_type
{
PLISQL_DTYPE_VAR,
PLISQL_DTYPE_ROW,
PLISQL_DTYPE_REC,
PLISQL_DTYPE_RECFIELD,
PLISQL_DTYPE_PROMISE,
PLISQL_DTYPE_PACKAGE_DATUM,
PLISQL_DTYPE_EXCEPTION,
}
|
Thank you so much for your PR! The user-defined EXCEPTION feature you've implemented is valuable and our team is quite interested in it. Before we can merge your code, according to our project's contribution guidelines, we need all contributors to sign a Contributor License Agreement (CLA). This is to ensure clear intellectual property rights for the project and to protect both the project and all contributors. You can find our contribution guidelines and CLA details here: Would you be willing to sign the CLA? If you agree, we'll guide you through the signing process. If you have any questions or concerns, please feel free to let us know. Thank you again for your excellent contribution! |
… codes Validates error codes at compile time per Oracle documentation: - Accept 100 or negative integers >= -1000000 (except -1403 and 0) - Reject with PLS-00701 for invalid codes
Sure, I'll sign the CLA as "Individual Contributor" and get back to you. [update] oh... your CLA is in paper form. Any chance for a digital CLA agent such as this? memgraph/helm-charts#166 (comment), makes individual contributions much easier. |
|
Thanks a lot for the contribution @rophy 🚀 Looks good to me |
Good proposal! We completely agree that a digital CLA system like CLA Assistant would be much more convenient for contributors. 👍 Your feedback is very valuable. We're currently evaluating digital CLA solutions (including CLA Assistant) and discussing the best approach for our project. This is definitely something we want to improve in the future. In the meantime, to avoid delaying the merge of your excellent PR #972, we'd like to propose the current available solution: CLA Signing Process:
Once we receive your signed CLA, we can proceed with reviewing and merging your PR without further delay. We really appreciate your patience and understanding. Your suggestion about digital CLA has been noted and is being considered by our team! Please let us know if you have any questions about the signing process. |
|
@yuanyl630 sent mail, thanks! |
https://docs.oracle.com/database/121/TTPLS/exceptions.htm#TTPLS191
Implement Oracle-compatible user-defined exception variables in PL/iSQL. Exceptions can be declared in packages and procedures, raised with RAISE, and caught with WHEN handlers.
Example:
CREATE PACKAGE BODY pkg IS custom_error EXCEPTION; PROCEDURE test IS BEGIN RAISE custom_error; EXCEPTION WHEN custom_error THEN RAISE INFO 'Caught exception'; END; END;Implementation stores exceptions as datums with unique dno, registered in namespace for lookup. Default SQLCODE is 1 for user-defined exceptions.
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.