From 5c2cf67a0251ac9cc2bd327b63fc601e129ee48c Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Thu, 1 Oct 2026 00:47:29 +0200 Subject: [PATCH] Bind the defaults a wrapper hands to the helper laying its state A set-returning wrapper can read an argument under a PG_NARGS guard and pass the local to a helper taking its FunctionCallInfo, which calls MEOS for the state the wrapper then streams. Tgeo_space_split reads ysize only when PG_NARGS() > 5 and zsize only when PG_NARGS() > 6, starting both from 0, and hands them to Tgeo_split_start, which calls tgeo_space_time_split_init. A helper parameter fed by such a local is bound on each signature omitting that argument, as a local the wrapper passes to MEOS itself is. A helper is recognised by its FunctionCallInfo parameter whatever it returns: void for Tgeo_split_start and Stbox_tiles_start, Datum for the helpers answering for their wrapper. The helper calls none of the members of the function group but their initialiser, which takes the members' parameters under the same names among others of its own. Each argument of that call binds to the member parameter of its name, as the generic twin a wrapper calls binds by name. The initialiser's parameters no member has, duration and torigin for tgeo_space_split, bind nothing. Why. A binding generating the short forms of a split or a tiling reads which sizes the call leaves out, and with what value, from boundArgs. The wrappers keep streaming the grid state; the catalog reads them as they are. Measured. Over MobilityDB 27c952dcc5, 34 SQL signatures gain boundArgs and none loses or changes one. 16 are the one-size and two-size forms of tgeo_space_split, tgeo_space_time_split, stbox_space_tiles and stbox_space_time_tiles, so every one-size signature of the ten functions taking ysize and zsize states {ysize: 0, zsize: 0}, every two-size one {zsize: 0}, and every full one nothing. The other 18 are the atStbox and minusStbox signatures of tgeoinst_restrict_stbox, tgeoseq_restrict_stbox and tgeoseqset_restrict_stbox, whose wrappers pass REST_AT and REST_MINUS through a helper: atfunc is REST_AT and REST_MINUS respectively. Witness. tests/test_boundargs.py binds each short form of a wrapper shaped as Tgeo_space_split to the sizes it omits, binds nothing for the initialiser's own parameters, for an initialiser sharing no parameter name with the member, or without the MEOS sources. The suite floor goes from 337 to 341. --- .github/workflows/pytest.yml | 2 +- parser/boundargs.py | 67 +++++++++++++--- tests/test_boundargs.py | 148 +++++++++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+), 14 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index df8e0b3..ca91be7 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -96,7 +96,7 @@ jobs: # carries, or a change to them is not exercised until after it merges. # Consumers use the action; this repository owns the rules. - name: Refuse a skip, and a suite that shrank - run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 337 + run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 341 # The rules earn their place by refusing a log that carries what they # name. Both fixtures are written here rather than tracked, and the diff --git a/parser/boundargs.py b/parser/boundargs.py index be97a53..cd30f2d 100644 --- a/parser/boundargs.py +++ b/parser/boundargs.py @@ -68,8 +68,10 @@ _NUMBER = re.compile(r"^-?\d+(?:\.\d+)?$") _ENUM = re.compile(r"^[A-Z][A-Z0-9_]+$") _IDENT = re.compile(r"^\w+$") -# A shared helper takes the call info plus the parameters the wrappers bind. -_HELPER = re.compile(r"Datum\s+(?P\w+)\s*\(\s*FunctionCallInfo\s+\w+" +# A shared helper takes the call info plus the parameters the wrappers bind, whatever it +# returns: a `Datum` helper answers for the wrapper, a `void` one lays the state a +# set-returning wrapper then streams (`Tgeo_split_start`, `Stbox_tiles_start`). +_HELPER = re.compile(r"\b(?P[A-Za-z_]\w*)\s*\(\s*FunctionCallInfo\s+\w+" r"(?P[^)]*)\)\s*\{") # ... and a wrapper delegates to it by passing that same call info straight through. _DELEG = re.compile(r"\b(?P\w+)\s*\(\s*fcinfo\s*(?P,[^;]*?)?\)\s*;") @@ -163,8 +165,11 @@ def extract_helpers(mdb_src: str | Path) -> dict[str, tuple[str, list[str]]]: def _delegated(body: str, helpers: dict[str, tuple[str, list[str]]]): - """``(helper_body, {helper_param: literal})`` when ``body`` delegates to a shared - helper, passing the call info through and binding the rest to literals.""" + """``(helper_body, {helper_param: literal}, {helper_param: (k, literal)})`` when + ``body`` delegates to a shared helper, passing the call info through: the helper + parameters the wrapper binds to literals, and those it binds to a local it reads from + argument ``k`` only when the call carries it (#_guarded_default), the literal the + local starts from being what a signature omitting argument ``k`` passes on.""" for m in _DELEG.finditer(body): entry = helpers.get(m.group("name")) if entry is None: @@ -172,14 +177,18 @@ def _delegated(body: str, helpers: dict[str, tuple[str, list[str]]]): hbody, hparams = entry raw = (m.group("args") or "").strip() vals = _split_args(raw[1:]) if raw.startswith(",") else [] - subst = {} + subst, gsubst = {}, {} for pname, val in zip(hparams, vals): lit = _literal(val) if lit is not None: subst[pname] = lit - if subst: - return hbody, subst - return None, {} + elif _IDENT.match(val): + dflt = _guarded_default(body, val) + if dflt is not None: + gsubst[pname] = dflt + if subst or gsubst: + return hbody, subst, gsubst + return None, {}, {} # A call to a lowercase C function, the form every MEOS function takes. @@ -283,13 +292,17 @@ def _guarded_default(body: str, var: str) -> tuple[int, str] | None: def _wrapper_bound(body: str, func: dict, drift: list, documented: dict[str, set], subst: dict[str, str] | None = None, - guarded: dict[str, tuple[int, str]] | None = None) -> dict[str, str]: + guarded: dict[str, tuple[int, str]] | None = None, + gsubst: dict[str, tuple[int, str]] | None = None) -> dict[str, str]: """The literals wrapper ``body`` binds in its call to ``func['name']``, keyed by ``func``'s parameter name. Empty if the wrapper does not call ``func`` by name. A local the wrapper reads from argument ``k`` only when the call carries it (``_guarded_default``) is caller-sourced for a signature stating ``k`` and a literal - for one omitting it; it goes into ``guarded`` as ``{param: (k, literal)}``. + for one omitting it; it goes into ``guarded`` as ``{param: (k, literal)}``. When + ``body`` is a helper the wrapper delegates to, ``gsubst`` holds the helper parameters + such a local reaches (#_delegated), and a call argument naming one of them goes into + ``guarded`` the same way. ``documented`` maps a MEOS function to the set of its ``@param``-documented parameter names (``parser.outparam.extract_param_names``). A bare-identifier argument bound to a @@ -301,6 +314,7 @@ def _wrapper_bound(body: str, func: dict, drift: list, if not args: return {} subst = subst or {} + gsubst = gsubst or {} assigned = {m.group("var") for m in _ASSIGNED.finditer(body)} doc_params = documented.get(func["name"], frozenset()) params = func.get("params", []) @@ -315,6 +329,10 @@ def _wrapper_bound(body: str, func: dict, drift: list, # a helper parameter the delegating wrapper bound to a literal bound[pname] = subst[a] continue + if a in gsubst and guarded is not None: + # a helper parameter the delegating wrapper feeds from a guarded local + guarded.setdefault(pname, gsubst[a]) + continue if a in assigned and _IDENT.match(a) and guarded is not None: dflt = _guarded_default(body, a) if dflt is not None: @@ -339,7 +357,14 @@ def _group_bound(body: str, group: list, helpers: dict, drift: list, from its call to whichever member of ``group`` it names (branches such as the RGEO ternary agree, and the first wins), or from its delegation to a shared helper when it names none, or from its call to the generic the members wrap when it does neither -- - a function none of them is whose parameter names, in ``generics``, equal a member's.""" + a function none of them is whose parameter names, in ``generics``, equal a member's. + + A helper can reach none of the members either: a set-returning wrapper hands its + arguments to a helper that lays the state it streams through the members' own + initialiser (`Tgeo_split_start` calls `tgeo_space_time_split_init`, not + `tgeo_space_split`). The initialiser takes the members' parameters under the same + names among others, so each argument of that call binds to the member parameter of + its name, as the generic twin binds by name.""" bound: dict[str, str] = {} guarded: dict[str, tuple[int, str]] = {} for func in group: @@ -350,12 +375,28 @@ def _group_bound(body: str, group: list, helpers: dict, drift: list, return bound, guarded # The wrapper names no MEOS call of its own: it delegates, and the literal it binds # sits at that delegation. - hbody, subst = _delegated(body, helpers) + hbody, subst, gsubst = _delegated(body, helpers) if hbody is not None: for func in group: for k, v in _wrapper_bound(hbody, func, drift, documented, subst, - guarded).items(): + guarded, gsubst).items(): bound.setdefault(k, v) + if not (bound or guarded) and generics: + member_params = {p.get("name") for f in group for p in f.get("params", [])} + names = {f["name"] for f in group} + for m in _CALLEE.finditer(hbody): + callee = m.group("name") + plist = generics.get(callee) + if callee in names or not plist or not set(plist) & member_params: + continue + twin = {"name": callee, + "params": [{"name": n if n in member_params else None} + for n in plist]} + for k, v in _wrapper_bound(hbody, twin, drift, documented, subst, + guarded, gsubst).items(): + bound.setdefault(k, v) + if bound or guarded: + break if bound or guarded or not generics: return bound, guarded # The wrapper calls a generic its typed members wrap: a function none of them is, diff --git a/tests/test_boundargs.py b/tests/test_boundargs.py index e4b4c76..59d965c 100644 --- a/tests/test_boundargs.py +++ b/tests/test_boundargs.py @@ -594,6 +594,154 @@ def test_computed_initializer_is_not_a_default(self): self.assertEqual(n, 0) +# Tgeo_space_split and Tgeo_split_start as mobilitydb/src/geo/tgeo_tile.c states them +HELPER_INIT_WRAPPERS = ''' +static void +Tgeo_split_start(FunctionCallInfo fcinfo, FuncCallContext *funcctx, + const Temporal *temp, double xsize, double ysize, double zsize, + const Interval *duration, const GSERIALIZED *sorigin, TimestampTz torigin, + bool bitmatrix, bool border_inc) +{ + int ntiles; + funcctx->user_fctx = tgeo_space_time_split_init(temp, xsize, ysize, zsize, + duration, sorigin, torigin, bitmatrix, border_inc, &ntiles); + get_call_result_type(fcinfo, 0, &funcctx->tuple_desc); + BlessTupleDesc(funcctx->tuple_desc); + return; +} + +Datum +Tgeo_space_split(PG_FUNCTION_ARGS) +{ + if (SRF_IS_FIRSTCALL()) + { + FuncCallContext *funcctx = SRF_FIRSTCALL_INIT(); + Temporal *temp = PG_GETARG_TEMPORAL_P(0); + double xsize = PG_GETARG_FLOAT8(1); + double ysize = 0; + double zsize = 0; + int i = 2; + if (PG_NARGS() > 5) + ysize = PG_GETARG_FLOAT8(i++); + if (PG_NARGS() > 6) + zsize = PG_GETARG_FLOAT8(i++); + GSERIALIZED *sorigin = PG_GETARG_GSERIALIZED_P(i++); + bool bitmatrix = PG_GETARG_BOOL(i++); + bool border_inc = PG_GETARG_BOOL(i++); + Tgeo_split_start(fcinfo, funcctx, temp, xsize, ysize, zsize, NULL, + sorigin, 0, bitmatrix, border_inc); + } + return Tgeo_split_next(fcinfo); +} + +static void +Other_start(FunctionCallInfo fcinfo, const Temporal *temp, double size) +{ + other_init(temp, size); +} + +Datum +Tgeo_other_split(PG_FUNCTION_ARGS) +{ + Temporal *temp = PG_GETARG_TEMPORAL_P(0); + double size = 0; + if (PG_NARGS() > 1) + size = PG_GETARG_FLOAT8(1); + Other_start(fcinfo, temp, size); + PG_RETURN_VOID(); +} +''' + +HELPER_INIT_MEOS = ''' +/** + * @brief Return the state of a split of a temporal value over a grid + */ +STboxGridState * +tgeo_space_time_split_init(const Temporal *temp, double xsize, double ysize, + double zsize, const Interval *duration, const GSERIALIZED *sorigin, + TimestampTz torigin, bool bitmatrix, bool border_inc, int *ntiles) +{ + return NULL; +} + +/** + * @brief Return the state of another split + */ +void * +other_init(const Temporal *value, double step) +{ + return NULL; +} +''' + + +class HelperInitTests(unittest.TestCase): + """A wrapper reading its sizes under PG_NARGS guards and handing them to a helper that + calls the members' initialiser, bound by parameter name as #GenericTwinTests binds a + generic.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + root = Path(self.tmp.name) + for sub, name, text in (("src", "tile.c", HELPER_INIT_WRAPPERS), + ("meos", "tile.c", HELPER_INIT_MEOS)): + (root / sub).mkdir() + (root / sub / name).write_text(text) + + def tearDown(self): + self.tmp.cleanup() + + def _split(self): + return {"name": "tgeo_space_split", "mdbC": "Tgeo_space_split", + "params": [{"name": n} for n in ( + "temp", "xsize", "ysize", "zsize", "sorigin", "bitmatrix", + "border_inc", "space_bins", "count")], + "sqlSignatures": [ + {"args": ["tgeompoint", "float", "float", "float", "geometry", + "boolean", "boolean"], "ret": "point_tpoint"}, + {"args": ["tgeompoint", "float", "geometry", "boolean", "boolean"], + "ret": "point_tpoint"}, + {"args": ["tgeompoint", "float", "float", "geometry", "boolean", + "boolean"], "ret": "point_tpoint"}]} + + def _merge(self, func, meos=True): + root = Path(self.tmp.name) + # the @param names run.py reads from the MEOS sources (parser.outparam) + documented = {"tgeo_space_time_split_init": { + "temp", "xsize", "ysize", "zsize", "duration", "sorigin", "torigin", + "bitmatrix", "border_inc", "ntiles"}} + return merge_boundargs({"functions": [func]}, root / "src", documented, + meos_src=root / "meos" if meos else None) + + def test_each_short_form_binds_the_sizes_it_omits(self): + idl, n, drift = self._merge(self._split()) + f = idl["functions"][0] + self.assertEqual([s.get("boundArgs") for s in f["sqlSignatures"]], + [None, {"ysize": "0", "zsize": "0"}, {"zsize": "0"}]) + self.assertNotIn("boundArgs", f.get("shape", {})) + self.assertEqual((n, drift), (3, [])) + + def test_a_literal_for_a_parameter_the_member_lacks_binds_nothing(self): + # duration NULL and torigin 0 reach the initialiser, which tgeo_space_split lacks + idl, _, _ = self._merge(self._split()) + for s in idl["functions"][0]["sqlSignatures"]: + self.assertFalse({"duration", "torigin"} & set(s.get("boundArgs") or {})) + + def test_an_initialiser_sharing_no_parameter_name_binds_nothing(self): + idl, n, _ = self._merge( + {"name": "tgeo_other_split", "mdbC": "Tgeo_other_split", + "params": [{"name": "temp"}, {"name": "width"}], + "sqlSignatures": [{"args": ["tgeompoint"], "ret": "tgeompoint"}]}) + f = idl["functions"][0] + self.assertFalse([s for s in f["sqlSignatures"] if "boundArgs" in s]) + self.assertNotIn("boundArgs", f.get("shape", {})) + + def test_without_the_meos_sources_the_initialiser_is_not_read(self): + idl, _, _ = self._merge(self._split(), meos=False) + f = idl["functions"][0] + self.assertFalse([s for s in f["sqlSignatures"] if "boundArgs" in s]) + + class BoundNameValueTests(unittest.TestCase): """A bound literal naming a macro of a header the parse did not read gets its value."""