Add two-argument and three-argument EXTRACT function forms to grammar#1222
Conversation
📝 WalkthroughWalkthroughAdds two new EXTRACT function forms to the Oracle parser grammar that accept two and three argument variants, producing explicit COERCE_EXPLICIT_CALL function calls. These rules are integrated into the existing func_expr_common_subexpr section of the expression grammar. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/backend/oracle_parser/ora_gram.y`:
- Around line 18028-18041: The parser creates unqualified function names for
EXTRACT in the two makeFuncCall sites (the list_make1(makeString("extract")) and
list_make2/3 variants) which can bind to user functions via search_path; update
those to use SystemFuncName("extract") instead so the calls are schema-qualified
like the other EXTRACT branches; modify the two occurrences in ora_gram.y where
makeString("extract") is passed into makeFuncCall to pass
SystemFuncName("extract") (keeping the same arg lists, COERCE_EXPLICIT_CALL, and
`@1`) so behavior matches existing extract handling.
- Around line 18035-18041: The grammar currently accepts a 3-argument EXTRACT
production that builds a call with makeFuncCall(... list_make3($3, $5, $7) ...),
but no corresponding cataloged extract signature exists; remove or disable the
three-argument alternative in ora_gram.y (the production that creates an
extract(...) via makeFuncCall and list_make3) so only the supported 2-arg
EXTRACT remains, or replace it with a clear syntax error action until a matching
runtime implementation (catalog signatures and resolver changes) is added;
ensure you update/remove the use of list_make3 and the COERCE_EXPLICIT_CALL path
associated with that production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7ccc4afb-ff73-44f3-881f-af76c2779606
⛔ Files ignored due to path filters (1)
contrib/ivorysql_ora/expected/ora_xml_functions.outis excluded by!**/*.out
📒 Files selected for processing (1)
src/backend/oracle_parser/ora_gram.y
| | EXTRACT '(' a_expr ',' a_expr ')' | ||
| { | ||
| $$ = (Node *) makeFuncCall(list_make1(makeString("extract")), | ||
| list_make2($3, $5), | ||
| COERCE_EXPLICIT_CALL, | ||
| @1); | ||
| } | ||
| | EXTRACT '(' a_expr ',' a_expr ',' a_expr ')' | ||
| { | ||
| $$ = (Node *) makeFuncCall(list_make1(makeString("extract")), | ||
| list_make3($3, $5, $7), | ||
| COERCE_EXPLICIT_CALL, | ||
| @1); | ||
| } |
There was a problem hiding this comment.
Use schema-qualified extract to avoid search_path-dependent binding
Lines 18030 and 18037 build an unqualified function name (makeString("extract")), while existing extract paths in this same block use SystemFuncName("extract"). This can resolve to user-defined functions unexpectedly and diverge from existing parser behavior.
Proposed consistency fix
- $$ = (Node *) makeFuncCall(list_make1(makeString("extract")),
+ $$ = (Node *) makeFuncCall(SystemFuncName("extract"),
list_make2($3, $5),
- COERCE_EXPLICIT_CALL,
+ COERCE_SQL_SYNTAX,
`@1`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/oracle_parser/ora_gram.y` around lines 18028 - 18041, The parser
creates unqualified function names for EXTRACT in the two makeFuncCall sites
(the list_make1(makeString("extract")) and list_make2/3 variants) which can bind
to user functions via search_path; update those to use SystemFuncName("extract")
instead so the calls are schema-qualified like the other EXTRACT branches;
modify the two occurrences in ora_gram.y where makeString("extract") is passed
into makeFuncCall to pass SystemFuncName("extract") (keeping the same arg lists,
COERCE_EXPLICIT_CALL, and `@1`) so behavior matches existing extract handling.
| | EXTRACT '(' a_expr ',' a_expr ',' a_expr ')' | ||
| { | ||
| $$ = (Node *) makeFuncCall(list_make1(makeString("extract")), | ||
| list_make3($3, $5, $7), | ||
| COERCE_EXPLICIT_CALL, | ||
| @1); | ||
| } |
There was a problem hiding this comment.
3-argument EXTRACT is accepted by grammar but has no guaranteed callable target
Line 18035 adds EXTRACT(a_expr, a_expr, a_expr) and rewrites to extract(...), but the available cataloged extract signatures shown are 2-arg variants. This will parse and then fail at function lookup/execution.
Proposed safe fix (remove unsupported 3-arg production until implementation exists)
- | EXTRACT '(' a_expr ',' a_expr ',' a_expr ')'
- {
- $$ = (Node *) makeFuncCall(list_make1(makeString("extract")),
- list_make3($3, $5, $7),
- COERCE_EXPLICIT_CALL,
- `@1`);
- }📝 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.
| | EXTRACT '(' a_expr ',' a_expr ',' a_expr ')' | |
| { | |
| $$ = (Node *) makeFuncCall(list_make1(makeString("extract")), | |
| list_make3($3, $5, $7), | |
| COERCE_EXPLICIT_CALL, | |
| @1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/oracle_parser/ora_gram.y` around lines 18035 - 18041, The grammar
currently accepts a 3-argument EXTRACT production that builds a call with
makeFuncCall(... list_make3($3, $5, $7) ...), but no corresponding cataloged
extract signature exists; remove or disable the three-argument alternative in
ora_gram.y (the production that creates an extract(...) via makeFuncCall and
list_make3) so only the supported 2-arg EXTRACT remains, or replace it with a
clear syntax error action until a matching runtime implementation (catalog
signatures and resolver changes) is added; ensure you update/remove the use of
list_make3 and the COERCE_EXPLICIT_CALL path associated with that production.
|
Would it work to use a User Define Function instead of patching grammar ? |
The C implementation already exists. What's missing is just the parser routing — two grammar rules to direct EXTRACT(expr, xpath) into the existing function. |
Summary by CodeRabbit
Fix for #1220