Conversation
Ported from SQLFluff b17ad001e338042f6dc78e3335cebbb8aa41b869 sqlfluff/sqlfluff#6973 sqlfluff/sqlfluff@b17ad00
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99d9f60f93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Ref::keyword("RETURNING").to_matchable(), | ||
| one_of(vec
Oracle's canonical JSON return type is VARCHAR2, so valid calls such as JSON_OBJECT('a' VALUE 1 RETURNING VARCHAR2(100)) cannot match this branch: the other alternatives only accept CLOB, BLOB, or JSON, leaving the entire function contents unparsable. Include VARCHAR2 alongside VARCHAR and apply the same size and character-semantics grammar.
Useful? React with 👍 / 👎.
| one_of(vec![ | ||
| Ref::new("StarSegment").to_matchable(), | ||
| Delimited::new(vec![Ref::new("JSONEntrySegment").to_matchable()]) | ||
| .to_matchable(), | ||
| ]) | ||
| .config(|config| { | ||
| config.optional(); | ||
| }) |
There was a problem hiding this comment.
When an empty object specifies an option, such as JSON_OBJECT(ABSENT ON NULL) or JSON_OBJECT(RETURNING JSON), this optional entry matcher greedily treats ABSENT or RETURNING as a shorthand expression entry because those words are valid naked identifiers. The following option clause then starts at ON or JSON and cannot match, so these valid option-only forms become unparsable; exclude the option-clause starters when matching the optional entry list.
Useful? React with 👍 / 👎.
| Ref::new("SingleIdentifierGrammar") | ||
| .optional() | ||
| .to_matchable(), |
There was a problem hiding this comment.
For valid forms such as JSON_OBJECT('a' VALUE 1 RETURNING CLOB STRICT), STRICT is not reserved in this dialect, so this optional SingleIdentifierGrammar greedily consumes it as a naked identifier. The outer STRICT clause consequently never matches, producing an incorrect syntax tree and preventing rules from recognizing the strict modifier; exclude STRICT from this optional identifier or model the actual trailing construct explicitly.
Useful? React with 👍 / 👎.
Benchmark for f62e099Click to view benchmark
|
1 participant
Summary
Ported from SQLFluff b17ad001e338042f6dc78e3335cebbb8aa41b869
sqlfluff/sqlfluff#6973
sqlfluff/sqlfluff@b17ad00
Validation
cargo fmt --all -- --checkcargo buildcargo testbazel test //... --test_timeout=600,300,900,3600 --local_test_jobs=1