get_data: fix SPI fetch order and portal leak on zero-row input

[AdeshDeshmukh]

Fixes #3146

Summary

Hardens the shared SPI fetch helper (pgget::get_data vector overload in
include/cpp_common/get_data.hpp) that all graph loaders (get_edges() +
~12 sibling getters) funnel through. Two defects, one focused patch, no
API/signature change.

What was wrong

1. The template read SPI_tuptable->tupdesc before checking SPI_processed,
   so a valid query returning zero rows (filtered-out edges_sql, empty
   tenant/partition) walked an unchecked path.
2. When column validation or row conversion threw, the open SPI cursor was
   never closed, leaking the portal.

This is distinct from the empty-string '' case (#3054), which is handled
upstream in the driver before the fetch loop.

Changes

• include/cpp_common/get_data.hpp (vector overload only; the legacy char*
  overload has no callers and is untouched):
  • Check SPI_processed first, with a defensive null guard on
    SPI_tuptable/tupdesc for PG-version variance.
  • On zero-row batches, validate columns when a descriptor is available so
    typos still raise Column 'cost' not Found instead of returning a
    misleading empty set; otherwise exit cleanly so drivers emit the standard
    NOTICE: No edges found.
  • Single try/catch around validate + iterate: tuple table freed and cursor
    closed exactly once before rethrowing.
• pgtap/dijkstra/dijkstra/no_crash_test.pg: +2 regression assertions
  (zero-row valid schema -> empty set; zero-row missing column -> XX000),
  plan counts adjusted accordingly.

Behavior after fix

• Zero rows + valid schema -> empty set + NOTICE, connection healthy.
• Zero rows + bad schema -> clean ERROR XX000, portal closed.
• '' empty string -> unchanged ERROR: Empty edges SQL.

Verification

• Patched header compiles (-std=c++17 -fsyntax-only, template
  instantiation checked)
• git diff --check clean; 4-space style, no lines >120
• Full pg_prove + live WHERE false repro need a PG+PostGIS box —
  requesting CI run for confirmation

Happy to adjust scope or split anything per reviewer preference.

Summary by CodeRabbit

• Bug Fixes

  • Prevented crashes when routing queries return no rows or lack expected columns.
  • Improved error handling for invalid empty-result schemas, including clear missing-column errors.
  • Ensured resources are cleaned up when processing errors occur.

• Tests

  • Added coverage for empty routing results and missing required columns.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4081420b-ea48-4a33-882f-f839c3b1e675

📥 Commits

Reviewing files that changed from the base of the PR and between 4712240 and 233a59a.

📒 Files selected for processing (2)

• include/cpp_common/get_data.hpp
• pgtap/dijkstra/dijkstra/no_crash_test.pg

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

pgget::get_data now handles zero-row SPI results without dereferencing missing tuple tables. It validates available schemas, cleans up resources during exceptions, and adds pgr_dijkstra regression tests for empty valid and invalid edge queries.

Changes

SPI fetch safety

Layer / File(s)	Summary
Zero-row fetch and schema validation include/cpp_common/get_data.hpp	The helper checks SPI_tuptable before reading its descriptor. It validates columns for empty batches when metadata exists, then exits cleanly.
Exception cleanup and regression coverage include/cpp_common/get_data.hpp, pgtap/dijkstra/dijkstra/no_crash_test.pg	The helper frees tuple tables and closes the SPI portal before rethrowing. Tests cover empty valid results and missing-column errors, and update the test plan counts.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 233a5

Empty edge queries now return safely while invalid schemas still report the expected error, with cleanup preserved on failures. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …	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 and concisely identifies the main changes: correcting SPI fetch ordering and preventing portal leaks for zero-row input.
Linked Issues check	✅ Passed	The PR meets the coding requirements in [#3146]. In pgget::get_data, the code reads SPI_processed before descriptor access and checks SPI_tuptable and tupdesc. It validates the schema for an a…
Out of Scope Changes check	✅ Passed	The reviewed changes stay within [#3146]. They modify the shared SPI fetch helper and add the two required regression assertions. The changes support zero-row handling, schema validation, resource cle…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the empty stream,
No dangling portal haunts the dream.
Columns speak when rows are none,
Errors rise, and leaks are gone.
Green tests hop beneath the sun.}

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