bug : Fragile SPI fetch order + portal leak on zero-row edges_sql in get_data.hpp

Open
  • Is something wrong?
#3,146 2 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 3 days

Nobody has claimed this yet.

  • #3147 by @AdeshDeshmukh — closed without merging

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
45/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp, postgresql
Domain
backend, databases

Research direction

The issue is in include/cpp_common/get_data.hpp in the pgget::get_data vector overload. Look at the shared SPI fetch helper. First, understand the SPI (Server Programming Interface) usage in PostgreSQL extensions. Check SPI_processed and SPI_tuptable for null before accessing the tuple descriptor. The fix involves adding defensive checks and exception-safe cleanup. Test with the provided SQL queries against the sample dataset. 'Done' means zero-row queries return cleanly or error without leaking resources, and existing tests pass.

Written by the indexing model from the issue text.

Description

bug Looks AI

Problem

A fragile sequence exists in the shared SPI fetch helper in include/cpp_common/get_data.hpp (the pgget::get_data vector overload). Specifically, the template inspects SPI_tuptable->tupdesc without first verifying whether any rows were returned (SPI_processed), and lacks an RAII/cleanup wrapper to close open cursors and release the tuple table if column validation or conversion throws.

In practice, executing a query that legitimately returns zero rows (e.g., WHERE false, an empty partition/tenant, or time-sliced edge filters with no active data) forces the execution through an unhandled edge case in what is intended to be the front door for graph loading. Because get_edges() in src/cpp_common/pgdata_getters.cpp and approximately a dozen sibling getters rely on this shared template, this affects all primary routing functions (pgr_dijkstra, pgr_bdDijkstra, etc.).

(Note: This is distinct from the empty string '' handling addressed in #3054 at shortestPath_driver.cpp:145-148, which operates upstream of the SPI execution loop).

Steps to Reproduce

Run the following against the standard pgTAP edge dataset (tools/testers/sampledata.pg):

-- 1. Zero rows, valid schema: should return empty set + NOTICE, never crash/hang
SELECT * FROM pgr_dijkstra(
    'SELECT id, source, target, cost FROM edges WHERE false',
    1, 2
);

-- 2. Zero rows, invalid schema (missing 'cost'): should raise clean column error, not leak/crash
SELECT * FROM pgr_dijkstra(
    'SELECT id, source FROM edges WHERE false',
    1, 2
);

-- 3. Sibling functions using the same template:
SELECT * FROM pgr_bdDijkstra(
    'SELECT id, source, target, cost, reverse_cost FROM edges WHERE false',
    1, 2, true
);

Expected Behavior

  • Zero rows + valid schema: Returns an empty result set and emits the existing NOTICE: "No edges found" (shortestPath_driver.cpp:221-225). The connection remains intact with zero resource leaks.
  • Zero rows + missing column: Raises a clean ERROR: Column 'cost' not Found (XX000 via standard message reporting) while closing any open cursor/portal. Typos should never be masked by an empty result or leave unclosed SPI state.
  • Empty string (''): Preserves existing behavior (ERROR: Empty edges SQL) with zero changes to public C/C++ function signatures.

Actual Behavior

  • Reading SPI_tuptable->tupdesc under zero-row conditions can lead to undefined behavior or segmentation faults depending on the Postgres server version.
  • When column validation throws an exception on missing columns, the open cursor/portal is never closed, leaking server resources.

Environment & Versions

  • OS: Ubuntu 24.04 (platform-independent)
  • PostgreSQL: 17.x (affects PG 13 through 18)
  • pgRouting: 4.1.0-dev (develop branch at commit 350b1e533)
  • PostGIS: Compatible with all supported versions

Proposed Fix

A focused, single-file patch in include/cpp_common/get_data.hpp preserving public API and ABI:

  1. Defensive Row & Descriptor Checks:
    Check SPI_processed and verify SPI_tuptable and SPI_tuptable->tupdesc against nullptr before accessing members.
  2. Schema Validation on Zero-Row Batches:
    If a valid tuple descriptor is available, validate required column headers even when row count is zero so that schema typos still raise an explicit ERROR: Column 'cost' not Found. If validation passes, exit cleanly to let downstream drivers trigger the standard NOTICE: No edges found.
  3. Guaranteed Resource Cleanup:
    Wrap column validation and row iteration in an exception-safe structure (or explicit try/catch) to ensure open cursors are closed and SPI buffers released prior to rethrowing errors.

Test Plan

Add the following regression checks to pgtap/dijkstra/dijkstra/no_crash_test.pg:

  • Zero rows with valid schema yields an empty set without server crashes.
  • Zero rows with missing mandatory columns raises XX000 with the expected error message.
Dominant language
C++
Stars
1.4k
Forks
397
Avg merge
2d 16h
Merged PRs (30d)
8

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from pgRouting/pgrouting

All issues in pgRouting/pgrouting

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.