Fix/remove v5 warnings by yasir-hussain-shah · Pull Request #1190 · IvorySQL/IvorySQL

yasir-hussain-shah · 2026-02-10T15:28:19Z

This PR fixes all the warnings in IVORY_REL_5_STABLE branch.

Summary by CodeRabbit

Bug Fixes
- Ensure timing baseline initializes correctly for accurate elapsed-time reporting.
- Fix string copy to guarantee null-termination and avoid unterminated name handling.
Refactor
- Streamlined function-removal and return-type validation logic for clarity.
- Simplified control flow in variable initialization/assignment to make match/error paths clearer.

coderabbitai · 2026-02-10T15:29:01Z

📝 Walkthrough

Walkthrough

Initialize psql timing baseline and simplify PL/PLiSQL control flow: set timing baseline in ExecQueryUsingCursor; replace flag-based matches with early returns/assertions in PL/PLiSQL package and subprocedure helpers; ensure memcpy copy in varlena string handling includes explicit NUL terminator.

Changes

Cohort / File(s)	Summary
psql timing `src/bin/psql/common.c`	Added `INSTR_TIME_SET_ZERO(before)` in `ExecQueryUsingCursor` to initialize timing baseline so elapsed_msec starts from zero when timing is enabled.
PL/PLiSQL — package logic `src/pl/plisql/src/pl_package.c`	In `plisql_remove_function_references` added an existence guard and early return/assertion before decrementing `use_count`; removed pre/post-length capture and related assertions. In `plisql_subproc_should_change_return_type` removed separate `found` flag and use `result` directly when detecting OUT/INOUT parameters.
PL/PLiSQL — subproc helpers `src/pl/plisql/src/pl_subproc_function.c`	In `plsql_init_glovalvar_from_stack` and `plsql_assign_out_glovalvar_from_stack` removed local `match` flag; functions now return immediately on successful match and assert/error if no match found, preserving behavior with simpler control flow.
Varlena string handling `src/backend/utils/adt/varlena.c`	Replaced `strncpy`-style in-place overwrite with `memcpy` and added explicit NUL terminator in `SplitGUCList` to guarantee null-termination when copy length equals source length.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

refactor subproc_function #917 — modifies PL/PLSQL subproc helper code similarly (related control-flow/assertion changes).
package phase II #945 — touches PL/PLSQL package and subprocedure logic akin to these edits.

Suggested reviewers

jiaoshuntian
NotHimmel

Poem

🐇 I nibble bugs with whiskered care,

Timers reset, flags float in air.
Early returns, tidy and spry,
Strings end proper — no stray byte to cry.
A little hop, the tests comply.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix/remove v5 warnings' accurately reflects the main objective of the pull request, which addresses and fixes warnings in the IVORY_REL_5_STABLE branch across multiple source files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into `IVORY_REL_5_STABLE`

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

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

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

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/pl/plisql/src/pl_package.c`:
- Around line 2732-2737: The code currently assumes via Assert that func is in
psource->source.funclist before removing it and decrementing
psource->source.use_count; replace that ASSERT-only protection with a runtime
check: call list_member_ptr(psource->source.funclist, func) and only perform
list_delete_ptr(psource->source.funclist, (void *) func) and
psource->source.use_count-- when the check returns true, otherwise skip the
delete/decrement (and optionally log or elog a warning/error) to avoid underflow
and use-after-free; refer to PLiSQL_package, psource, funclist, use_count,
list_member_ptr, list_delete_ptr and func to locate the change.

🧹 Nitpick comments (1)

src/bin/psql/common.c (1)
2897-2899: Use INSTR_TIME_SET_ZERO() for consistency with the opaque instr_time API.

On Line 2898, before.ticks = 0 directly accesses struct fields. The instr_time type is documented as opaque and should only be manipulated via provided macros. Using INSTR_TIME_SET_ZERO(before) maintains API abstraction and is consistent with the same function's initialization pattern at lines 1456 and 1676.
🔧 Suggested fix
 *elapsed_msec = 0;
-before.ticks = 0;
+INSTR_TIME_SET_ZERO(before);

use INSTR_TIME_SET_ZERO instead of setting ticks directly

jiaoshuntian · 2026-02-11T06:41:14Z

Thanks for cleaning up the code warnings.

…h of the source argument

yasir-hussain-shah added 3 commits February 10, 2026 17:03

fix warnings: variable ‘match’ set but not used

e40a4ac

fix warning: unused variable pre_len

3633126

fix warning: variable ‘found’ set but not used

9c42b86

coderabbitai Bot reviewed Feb 10, 2026

View reviewed changes

Comment thread src/pl/plisql/src/pl_package.c

yasir-hussain-shah added 2 commits February 10, 2026 23:03

fix warning: ‘before.ticks’ may be used uninitialized

9f5e9a7

use INSTR_TIME_SET_ZERO instead of setting ticks directly

don't attempt to delete from list if item doesn't belong to it

1e335b2

yasir-hussain-shah force-pushed the fix/remove-v5-warnings branch from 50b0abe to 1e335b2 Compare February 10, 2026 18:13

fix warning: ‘__builtin_strncpy’ specified bound depends on the lengt…

b822be0

…h of the source argument

gaoxueyu merged commit 30527cc into IvorySQL:IVORY_REL_5_STABLE Feb 14, 2026
6 checks passed

gaoxueyu added this to IvorySQL 5.3 Mar 12, 2026

github-project-automation Bot moved this to Done in IvorySQL 5.3 Mar 12, 2026

yuanyl630 mentioned this pull request Mar 19, 2026

Remove all the warnings in IVORY_REL_5_STABLE branch #1250

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/remove v5 warnings#1190

Fix/remove v5 warnings#1190
gaoxueyu merged 6 commits into
IvorySQL:IVORY_REL_5_STABLEfrom
yasir-hussain-shah:fix/remove-v5-warnings

yasir-hussain-shah commented Feb 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Feb 10, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

jiaoshuntian commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yasir-hussain-shah commented Feb 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jiaoshuntian commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yasir-hussain-shah commented Feb 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Feb 10, 2026 •

edited

Loading