Fix PL/pgSQL IF hang and bump tree-sitter-postgres to 1.2.2 by gmr · Pull Request #8 · gmr/libpgfmt

gmr · 2026-06-15T15:38:49Z

Summary

Fixes two issues surfaced by production DDL that libpgfmt couldn't handle.

1. PL/pgSQL `IF` infinite-loop hang

format_stmt_if walked the statement's named children in a while loop but only advanced the index for the opening kw_if. The closing kw_if of END IF hit the same match arm, fell through without incrementing, and the loop spun forever. Any format_plpgsql body containing an IF statement hung.

Fix: advance the index for the closing kw_if. Added tests/plpgsql_test.rs covering IF/THEN/END IF and IF/ELSIF/ELSE (these would hang before the fix; the existing create_function fixture didn't catch it because the SQL path treats the function body as opaque text).

2. Decimal literals corrupted (`0.00` -> `0 .00`)

tree-sitter-postgres 1.2.1 had a float_literal lexer bug (stray trailing space in the regex) that split decimals like 0.00 into 0 (an ERROR node) + .00. libpgfmt then emitted invalid SQL — (0.00)::real -> (0 .00)::real and THEN 0.0 -> THEN .0.

Fix: bump tree-sitter-postgres to 1.2.2 (gmr/tree-sitter-postgres#41), which corrects the lexer. The affected production views now parse with zero ERROR nodes and format to valid SQL.

Also adds format_plpgsql_test and dump_plpgsql example binaries for exercising the PL/pgSQL path, mirroring the existing SQL examples.

Verification

just check (fmt + clippy + test) passes; all test suites green.
The previously-hanging production function now formats and terminates.
0.00::real, 0.0, 12.5e3, .5 all format correctly with no ERROR nodes.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Fixed PL/pgSQL IF/END IF block formatting that was causing incorrect behavior.
New Features
- Added example programs demonstrating PL/pgSQL file parsing and formatting workflows.
Tests
- Added regression tests for PL/pgSQL IF statement formatting.

format_stmt_if never advanced its child index for the closing `kw_if` of `END IF`, so any `format_plpgsql` body containing an IF statement looped forever. Add the missing increment and cover it with regression tests (IF/THEN and IF/ELSIF/ELSE). Bump tree-sitter-postgres to 1.2.2, which fixes a float_literal lexer bug that split decimals like `0.00` into `0` + `.00`. That previously corrupted output (`(0.00)::real` -> `(0 .00)::real`, `0.0` -> `.0`); the affected production views now parse cleanly. Add example binaries for exercising the PL/pgSQL path (format_plpgsql_test, dump_plpgsql), mirroring the existing SQL examples. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

gmr · 2026-06-15T15:44:06Z

PR monitoring complete — this PR is green and ready to merge.

CI status:

test (stable): pass (32s)
test (nightly): pass (44s)

Merge state: MERGEABLE / CLEAN.

No automated review feedback to address: this repository has no automated reviewer (CodeRabbit, Claude Code Review, etc.) installed, and there are no open review threads or comments on the PR.

No code changes were required during monitoring. Ready for human review/merge.

gmr · 2026-06-15T15:54:52Z

@coderabbitai review

coderabbitai · 2026-06-15T15:55:00Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-06-15T15:55:06Z

📝 Walkthrough

Walkthrough

Fixes an infinite loop in format_stmt_if where the END IF closing token was not advancing the loop index, causing the formatter to hang. Two regression tests are added to cover IF...END IF and IF...ELSIF...ELSE...END IF constructs. The tree-sitter-postgres dependency is bumped to 1.2.2, and two example binaries are added for syntax tree inspection and format testing.

Changes

PL/pgSQL formatter fix and tooling

Layer / File(s)	Summary
END IF index advancement fix and regression tests `src/formatter/plpgsql.rs`, `tests/plpgsql_test.rs`, `Cargo.toml`	`format_stmt_if` now increments `i` when encountering a non-opening `kw_if` token to skip the `END IF` closing token. Regression tests assert correct formatted output for plain and branched `IF` blocks. `tree-sitter-postgres` bumped to `1.2.2`.
New dump and format example binaries `examples/dump_plpgsql.rs`, `examples/format_plpgsql_test.rs`	`dump_plpgsql.rs` loads a PL/pgSQL file, parses it with Tree-sitter, and recursively prints the syntax tree. `format_plpgsql_test.rs` reads a file and style from CLI args, calls `format_plpgsql`, prints the result, and exits non-zero on error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A loop spun on forever, oh dear!
The END IF kept coming, year after year.
One i += 1, so small yet so bright,
Now the formatter halts — everything right.
I nibble my carrot, the tests all go green! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 clearly summarizes the two main changes: fixing a PL/pgSQL IF hang and updating tree-sitter-postgres, which align with the PR's core objectives.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@examples/dump_plpgsql.rs`:
- Line 7: The short variable assignment on line 7 uses byte slicing with [..60]
which can panic if it cuts through a multibyte UTF-8 character. Replace this
byte-level truncation with character-safe truncation by iterating through the
string's characters and truncating after the 60th character, or by finding the
byte boundary that corresponds to the 60th character to ensure the slice always
ends at a valid UTF-8 boundary.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e636f3b5-bbdf-4fb7-beca-cdb53bff219c

📥 Commits

Reviewing files that changed from the base of the PR and between 3b077c0 and abed675.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

Cargo.toml
examples/dump_plpgsql.rs
examples/format_plpgsql_test.rs
src/formatter/plpgsql.rs
tests/plpgsql_test.rs

coderabbitai · 2026-06-15T15:59:09Z

+fn print_tree(node: tree_sitter::Node, source: &str, indent: usize) {
+    let kind = node.kind();
+    let text = &source[node.byte_range()];
+    let short = if text.len() > 60 { &text[..60] } else { text };


⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd /tmp && find . -name "dump_plpgsql.rs" -type f 2>/dev/null | head -5

Repository: gmr/libpgfmt

Length of output: 38

🏁 Script executed:

git ls-files examples/dump_plpgsql.rs

Repository: gmr/libpgfmt

Length of output: 81

🏁 Script executed:

cat -n examples/dump_plpgsql.rs | head -20

Repository: gmr/libpgfmt

Length of output: 822

Use UTF-8-safe truncation for preview text.

Line 7 can panic on valid SQL containing multibyte characters because byte slicing ([..60]) may cut through a UTF-8 code point.

Proposed fix

- let short = if text.len() > 60 { &text[..60] } else { text }; + let short = if text.len() > 60 { text.chars().take(60).collect::<String>() } else { text.to_string() }; let short = short.replace('\n', "\\n");

📝 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.

Suggested change

let short = if text.len() > 60 { &text[..60] } else { text };

let short = if text.len() > 60 { text.chars().take(60).collect::<String>() } else { text.to_string() };

let short = short.replace('\n', "\\n");

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/dump_plpgsql.rs` at line 7, The short variable assignment on line 7 uses byte slicing with [..60] which can panic if it cuts through a multibyte UTF-8 character. Replace this byte-level truncation with character-safe truncation by iterating through the string's characters and truncating after the 60th character, or by finding the byte boundary that corresponds to the 60th character to ensure the slice always ends at a valid UTF-8 boundary.

coderabbitai Bot reviewed Jun 15, 2026

View reviewed changes

gmr merged commit 63535ab into main Jun 15, 2026
3 checks passed

gmr deleted the fix/plpgsql-if-hang branch June 15, 2026 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix PL/pgSQL IF hang and bump tree-sitter-postgres to 1.2.2#8

Fix PL/pgSQL IF hang and bump tree-sitter-postgres to 1.2.2#8
gmr merged 1 commit into
mainfrom
fix/plpgsql-if-hang

gmr commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

gmr commented Jun 15, 2026

Uh oh!

gmr commented Jun 15, 2026

Uh oh!

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	let short = if text.len() > 60 { &text[..60] } else { text };
	let short = if text.len() > 60 { text.chars().take(60).collect::<String>() } else { text.to_string() };
	let short = short.replace('\n', "\\n");

Conversation

gmr commented Jun 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. PL/pgSQL IF infinite-loop hang

2. Decimal literals corrupted (0.00 -> 0 .00)

Verification

Summary by CodeRabbit

Uh oh!

gmr commented Jun 15, 2026

Uh oh!

gmr commented Jun 15, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

gmr commented Jun 15, 2026 •

edited by coderabbitai Bot

Loading

1. PL/pgSQL `IF` infinite-loop hang

2. Decimal literals corrupted (`0.00` -> `0 .00`)

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading

coderabbitai Bot commented Jun 15, 2026 •

edited

Loading