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

Abierto
  • ¿Algo no está bien?
#3,146 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

Nadie ha tomado este issue todavía.

  • #3147 de @AdeshDeshmukh — cerrado sin fusionar

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
45/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
cpp, postgresql
Área
backend, databases

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.
Lenguaje dominante
C++
Estrellas
1.4k
Forks
397
Merge medio
2 d 2 h
PR fusionados (30 d)
4

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de pgRouting/pgrouting

Todos los issues de pgRouting/pgrouting

Issues similares

Más issues de C++

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.