From b5e17ea5c63b54f62a368dcd7ebd201cd1a80fd8 Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Fri, 9 Oct 2026 16:12:23 +0200 Subject: [PATCH] Bind NULL for a nullable parameter a C wrapper derives from another argument A parameter MEOS documents as possibly NULL that the PostgreSQL wrapper fills from a value it derives from an SQL argument another parameter already carries, and not from an SQL argument of its own, is bound to NULL for a binding: asMFJSON(tgeompoint, ...), whose wrapper Temporal_as_mfjson names the coordinate reference system with srs = get_srs_cache_by_srid(fcinfo, srid, ...) from the SRID of its temporal argument and calls temporal_as_mfjson(temp, ..., srs), states srs as NULL, so a binding calls the function with NULL and MEOS names the system from its own reference systems. A local assigned from an SQL argument, read by PG_GETARG under a running index or converted from an array, and a state threaded through the call's own result keep carrying their argument. Witness: on MobilityDB master 8a9bc20b62 temporal_as_mfjson documents srs as possibly NULL, its wrapper derives srs from the SRID of the temporal value through the table spatial_ref_sys, and master's parser classifies srs as a local the caller supplies, so the catalog states it as an SQL argument the twenty asMFJSON signatures do not carry. Measured: over MobilityDB master 8a9bc20b62 run.py states srs as NULL on 20 of the 20 signatures of temporal_as_mfjson, and every other key of the catalog equals that of master's parser. The suite passes the new tests of tests/test_boundargs.py HostDerivedNullableTests, the first of which fails with master's parser while the three stating that an argument read by a running index, an array converted into two parameters and an aggregate state stay unbound pass with both, with 432 passed and 84 skipped against 428 and 84 with master's parser. Why: a binding calls the C function with the parameters the SQL function carries, and leaving the derived one to MEOS gives it the name MEOS states for the SRID of the value. --- parser/boundargs.py | 40 ++++++++++++++++++ tests/test_boundargs.py | 91 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/parser/boundargs.py b/parser/boundargs.py index 60c9990..dabf623 100644 --- a/parser/boundargs.py +++ b/parser/boundargs.py @@ -344,6 +344,20 @@ def _wrapper_bound(body: str, func: dict, drift: list, if dflt is not None: guarded.setdefault(pname, dflt) continue + if (a in assigned and _IDENT.match(a) + and pname in ((func.get("shape") or {}).get("nullable") or ()) + and not _assigned_from_argument(body, a) + and not re.search(r"(?])" + re.escape(a) + r"\s*=(?!=)[^;]*\b" + + re.escape(func["name"]) + r"\s*\(", body) + and _caller_index(body, a) in _directly_carried( + body, [o for o in args if o != a], assigned)): + # a value the host derives from an SQL argument another parameter carries + # itself, for a parameter the function reads as NULL where the caller gives + # none, as Temporal_as_mfjson fills srs from the table spatial_ref_sys by the + # SRID of the temporal value temp carries: a binding passes NULL and the + # function derives it itself + bound[pname] = "NULL" + continue if a.startswith("&") or "PG_GETARG" in a or a in assigned: continue # out-param or caller-sourced local lit = _literal(a) @@ -532,6 +546,32 @@ def _direct_indices(expr: str) -> set[int]: return {int(a or b) for a, b in _GETARG.findall(expr)} +def _assigned_from_argument(body: str, var: str) -> bool: + """True when an assignment of the local ``var`` in the wrapper ``body`` reads an SQL + argument itself, as ``option = PG_GETARG_INT32(1)`` does, rather than deriving its value + from other locals, as ``srs = get_srs_cache_by_srid(fcinfo, srid, true)`` does: the first + step #_caller_index takes, without the walk through other locals it then makes. An + argument read by a running index, as ``sorigin = PG_GETARG_GSERIALIZED_P(i++)``, reads + one too.""" + rhs = re.finditer(r"(?])" + re.escape(var) + r"\s*=(?!=)\s*([^;]+);", body) + return any(_direct_indices(m.group(1)) or "PG_GETARG" in m.group(1) for m in rhs) + + +def _directly_carried(body: str, args: list[str], assigned: set) -> set[int]: + """The SQL arguments the call arguments ``args`` of a wrapper ``body`` carry themselves: + an argument reading ``PG_GETARG_(k)``, or a local an assignment of which reads it, + as #_assigned_from_argument reads it, and not a value derived from such a local.""" + out: set[int] = set() + for o in args: + o = o.strip() + if "PG_GETARG" in o or (o in assigned and _IDENT.match(o) + and _assigned_from_argument(body, o)): + k = _caller_index(body, o) + if k is not None: + out.add(k) + return out + + def _caller_index(body: str, arg: str, depth: int = 0) -> int | None: """The SQL argument the call argument ``arg`` of a wrapper ``body`` carries, or None. diff --git a/tests/test_boundargs.py b/tests/test_boundargs.py index 1433bc6..1f17495 100644 --- a/tests/test_boundargs.py +++ b/tests/test_boundargs.py @@ -404,6 +404,97 @@ def test_undocumented_param_drifts(self): ''' +DERIVED = r""" +Datum +Temporal_as_mfjson(PG_FUNCTION_ARGS) +{ + int option = 0; + char *srs = NULL; + Temporal *temp = PG_GETARG_TEMPORAL_P(0); + if (PG_NARGS() > 1 && ! PG_ARGISNULL(1)) + option = PG_GETARG_INT32(1); + int32_t srid = tspatial_srid(temp); + if (srid != SRID_UNKNOWN) + srs = get_srs_cache_by_srid(fcinfo, srid, true); + char *mfjson = temporal_as_mfjson(temp, option, srs); + PG_RETURN_TEXT_P(cstring_to_text(mfjson)); +} + +Datum +Tgeo_space_boxes(PG_FUNCTION_ARGS) +{ + Temporal *temp = PG_GETARG_TEMPORAL_P(0); + int i = 1; + GSERIALIZED *sorigin = PG_GETARG_GSERIALIZED_P(i++); + STBox *boxes = tgeo_space_boxes(temp, sorigin); + PG_RETURN_POINTER(boxes); +} + +Datum +Raster_clip(PG_FUNCTION_ARGS) +{ + Raster *rast = PG_GETARG_RASTER_P(0); + ArrayType *array = PG_GETARG_ARRAYTYPE_P(1); + int nbands; + int *bands = intarr_extract(array, &nbands); + Raster *result = raster_clip(rast, bands, nbands); + PG_RETURN_POINTER(result); +} + +Datum +Temporal_tcount_transfn(PG_FUNCTION_ARGS) +{ + SkipList *state; + INPUT_AGG_TRANS_STATE(fcinfo, state, ctx); + Temporal *temp = PG_GETARG_TEMPORAL_P(1); + state = temporal_tcount_transfn(state, temp); + PG_RETURN_POINTER(state); +} +""" + + +def _derived_idl(): + def fn(name, wrapper, params, nullable): + return {"name": name, "mdbC": wrapper, "params": [{"name": p} for p in params], + "shape": {"nullable": nullable}} + return {"functions": [ + fn("temporal_as_mfjson", "Temporal_as_mfjson", ["temp", "option", "srs"], ["srs"]), + fn("tgeo_space_boxes", "Tgeo_space_boxes", ["temp", "sorigin"], ["sorigin"]), + fn("raster_clip", "Raster_clip", ["rast", "bands", "nbands"], ["bands"]), + fn("temporal_tcount_transfn", "Temporal_tcount_transfn", ["state", "temp"], + ["state"])]} + + +class HostDerivedNullableTests(unittest.TestCase): + """A nullable parameter the wrapper fills from a value it derives from an SQL argument + another parameter carries itself is NULL for a binding; one carrying an SQL argument of + its own, or threaded through the call, is not.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + src = Path(self.tmp.name) / "src" + src.mkdir() + (src / "derived.c").write_text(DERIVED) + self.idl, _, _ = merge_boundargs(_derived_idl(), self.tmp.name) + self.bound = {f["name"]: (f.get("shape") or {}).get("boundArgs") + for f in self.idl["functions"]} + + def tearDown(self): + self.tmp.cleanup() + + def test_a_name_the_host_derives_is_null_for_a_binding(self): + self.assertEqual(self.bound["temporal_as_mfjson"], {"srs": "NULL"}) + + def test_an_argument_read_by_a_running_index_is_carried(self): + self.assertIsNone(self.bound["tgeo_space_boxes"]) + + def test_an_array_argument_derived_into_two_parameters_is_carried(self): + self.assertIsNone(self.bound["raster_clip"]) + + def test_a_state_threaded_through_the_call_is_not_bound(self): + self.assertIsNone(self.bound["temporal_tcount_transfn"]) + + class SiblingWrapperTests(unittest.TestCase): """One MEOS function behind two wrappers, one per SQL signature."""