From af509689697586c79c638dd03c9d50a5eec67bf8 Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Fri, 14 Aug 2026 23:37:18 +0200 Subject: [PATCH] Give a function the SQL surface of every wrapper it backs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A MEOS function often backs several PostgreSQL wrappers, each registering its own CREATE FUNCTION overloads: the ever/always pair over one `ea_*` kernel, the shift/scale/shiftScale trio over one `*_shift_scale`, send and asBinary over one `*_as_wkb`, and the argument-commuted form of an asymmetric operation (`NAD_stbox_tgeo` beside `NAD_tgeo_stbox`, both over `nad_tgeo_stbox`). `sqlSignatures` read only the first of them, so every sibling wrapper's overloads were dropped — the always half of each ever/always pair, two thirds of each shift/scale trio, and any commuted argument order. Take the union over all of them instead, scope-filtering each wrapper on its own and de-duplicating. A wrapper is shared whenever two functions name it, in whatever position, so gather the claimants of every wrapper rather than of the first alone; that brings `Temporal_scale_time` under the scope check, and its generic claimant is declared alongside its two siblings. PostgreSQL also accepts the function attributes in any order, and `aTouches(tcbuffer, cbuffer)` writes SUPPORT before the body, which parsed as the return type `boolean SUPPORT tspatial_supportfn`. Keep only the type. Deriving from MobilityDB 071d4c80de leaves the 5537 functions unchanged and takes sqlSignatures from 8234 to 9027: 111 functions gain 793 overloads and none loses any. --- meta/type-scope.json | 4 + parser/sqlfn.py | 62 ++++++++++--- tests/test_sqlfn_wrappers.py | 166 +++++++++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 11 deletions(-) create mode 100644 tests/test_sqlfn_wrappers.py diff --git a/meta/type-scope.json b/meta/type-scope.json index 4322348..7bd3678 100644 --- a/meta/type-scope.json +++ b/meta/type-scope.json @@ -34,6 +34,10 @@ "types": "*", "note": "Generic over Temporal *: sets the interpolation of any temporal type." }, + "temporal_scale_time": { + "types": "*", + "note": "Generic over Temporal *: scales the time of any temporal type." + }, "temporal_shift_time": { "types": "*", "note": "Generic over Temporal *: shifts the time of any temporal type." diff --git a/parser/sqlfn.py b/parser/sqlfn.py index d99521c..190b311 100644 --- a/parser/sqlfn.py +++ b/parser/sqlfn.py @@ -52,6 +52,10 @@ # binds is in the trailing `AS 'MODULE_PATHNAME', ''`. _CREATE_FN = re.compile(r"CREATE\s+(?:OR\s+REPLACE\s+)?FUNCTION\s+(\w+)\s*\(", re.I) _AS_WRAPPER = re.compile(r"AS\s+'[^']*'\s*,\s*'(\w+)'", re.I) +# A CREATE FUNCTION attribute that may follow RETURNS before the body. +_RET_ATTR = re.compile( + r"\b(?:SUPPORT|LANGUAGE|WINDOW|IMMUTABLE|STABLE|VOLATILE|LEAKPROOF|CALLED|RETURNS\s+NULL|" + r"STRICT|SECURITY|PARALLEL|COST|ROWS|TRANSFORM|SET)\b", re.I) def _split_top_commas(s): @@ -128,6 +132,13 @@ def _create_fn_stmts(text): wrapper = wm.group(1) if wm else None rm = re.match(r"\s*RETURNS\s+(?:SETOF\s+)?(.+?)\s+AS\b", tail, re.I | re.S) ret = " ".join(rm.group(1).split()) if rm else None + if ret: + # PostgreSQL lets the function attributes come in any order, so an + # attribute may sit between RETURNS and AS rather than after the body. + # MobilityDB writes SUPPORT after `AS 'MODULE_PATHNAME'` everywhere but + # `aTouches(tcbuffer, cbuffer)`, which puts it first and so parsed as the + # return type `boolean SUPPORT tspatial_supportfn`. Keep only the type. + ret = _RET_ATTR.split(ret, maxsplit=1)[0].strip() or ret argdecls = [a for a in _split_top_commas(text[start:arg_close]) if a.strip()] yield sqlname, argdecls, ret, wrapper @@ -307,9 +318,13 @@ def attach_sqlfn_map(idl, meos_src, mdb_src, sql_src=None): for f in idl["functions"]: if f.get("api") != "public": continue + # EVERY wrapper the function claims, not just the first: a wrapper is + # shared whenever two functions name it, in whatever position. Reading + # only the first left a wrapper claimed second by everyone (Numset_scale, + # named after Numset_shift by all five numeric set types) out of + # `shared_wrappers`, so its signatures went unfiltered. for w in m2d.get(f["name"]) or (): claimed.setdefault(w, []).append(f["name"]) - break shared_wrappers = {w for w, names in claimed.items() if len(names) > 1 and len(w2sig.get(w) or ()) > 1} require_scopes([n for w in shared_wrappers for n in claimed[w]], @@ -348,17 +363,42 @@ def attach_sqlfn_map(idl, meos_src, mdb_src, sql_src=None): # The SQL-facing arity (required..total). Lets a generator expose the SQL # signature instead of the wider C one: args beyond sqlArity are SQL-optional # (DEFAULT), and C params beyond sqlArityMax are C-only out-params. - sigs = w2sig.get(wrappers[0]) - # Only a public claimant of a shared wrapper is filtered: those are the - # functions a binding projects, and the ones require_scopes has proven a - # scope for. An internal function is not part of any binding surface. + # The registration surface is the union over EVERY wrapper the function + # claims, not the first one's alone. One MEOS function commonly backs a + # whole SET of wrappers — the ever/always pair (eDwithin + aDwithin over + # one `ea_dwithin_*`), the shift/scale/shiftScale trio over one + # `*_shift_scale`, send + asBinary over one `*_as_wkb` — and each wrapper + # registers its OWN CREATE FUNCTION overloads. Keeping only `wrappers[0]` + # dropped every sibling wrapper's overloads, so half of each ever/always + # pair and two thirds of each shift/scale trio were invisible to bindings. + # It is also what made a COMMUTED wrapper unrepresentable: `NAD_stbox_tgeo` + # is a second wrapper over the one `nad_tgeo_stbox`, so even a correct + # `@csqlfn #NAD_tgeo_stbox() #NAD_stbox_tgeo()` could not have carried the + # argument-swapped overload through. + # Each wrapper is scope-filtered on its own — a scope answers "which types + # does this function serve", which is per wrapper — and the union is + # de-duplicated, since two wrappers may legitimately register the same + # overload under the same name. scoped = False - if sigs and wrappers[0] in shared_wrappers and f.get("api") == "public": - scope, _ = resolve_scope(f["name"], scope_facts, scope_bodies, - scope_params, declared) - if scope is not None: - sigs = signatures_for(f["name"], sigs, scope) - scoped = True + sigs, seen = [], set() + for w in wrappers: + wsigs = w2sig.get(w) + if not wsigs: + continue + # Only a public claimant of a shared wrapper is filtered: those are the + # functions a binding projects, and the ones require_scopes has proven a + # scope for. An internal function is not part of any binding surface. + if w in shared_wrappers and f.get("api") == "public": + scope, _ = resolve_scope(f["name"], scope_facts, scope_bodies, + scope_params, declared) + if scope is not None: + wsigs = signatures_for(f["name"], wsigs, scope) + scoped = True + for s in wsigs: + key = (s["sqlName"], tuple(s["args"]), s["ret"]) + if key not in seen: + seen.add(key) + sigs.append(s) if sigs: f["sqlArity"] = min(s["required"] for s in sigs) f["sqlArityMax"] = max(len(s["args"]) for s in sigs) diff --git a/tests/test_sqlfn_wrappers.py b/tests/test_sqlfn_wrappers.py new file mode 100644 index 0000000..1bd6c1e --- /dev/null +++ b/tests/test_sqlfn_wrappers.py @@ -0,0 +1,166 @@ +"""A MEOS function's SQL surface is the union over every wrapper it claims. + +One MEOS function commonly backs a SET of PostgreSQL wrappers, each registering +its own CREATE FUNCTION overloads: the ever/always pair over one `ea_*` kernel, +the shift/scale/shiftScale trio over one `*_shift_scale`, send + asBinary over +one `*_as_wkb`, and the argument-COMMUTED form of an asymmetric operation +(`NAD_stbox_tgeo` beside `NAD_tgeo_stbox`, both over `nad_tgeo_stbox`). Reading +only the first wrapper dropped every sibling's overloads, so half of each +ever/always pair — and any commuted overload — was invisible to bindings. + +Plain unittest, no pytest dependency; synthetic sources via a temp dir. +""" +import tempfile +import unittest +from pathlib import Path + +from parser.sqlfn import _create_fn_stmts, attach_sqlfn_map + +MEOS_C = """ +/** + * @ingroup meos_geo_distance + * @brief Return the nearest approach distance between a temporal geo and a box + * @csqlfn #NAD_tgeo_stbox() #NAD_stbox_tgeo() + */ +double +nad_tgeo_stbox(const Temporal *temp, const STBox *box) +{ +} + +/** + * @ingroup meos_geo_rel + * @brief Return true if a temporal geo and a geo are ever or always within a + * distance of each other + * @csqlfn #Edwithin_tgeo_geo() #Adwithin_tgeo_geo() + */ +int +ea_dwithin_tgeo_geo(const Temporal *temp, const GSERIALIZED *gs, double dist, + bool ever) +{ +} +""" + +MDB_C = """ +/** + * @brief Return the nearest approach distance between a temporal geo and a box + * @sqlfn nearestApproachDistance() + * @sqlop @p |=| + */ +Datum +NAD_tgeo_stbox(PG_FUNCTION_ARGS) +{ +} + +/** + * @brief Return the nearest approach distance between a box and a temporal geo + * @sqlfn nearestApproachDistance() + * @sqlop @p |=| + */ +Datum +NAD_stbox_tgeo(PG_FUNCTION_ARGS) +{ +} + +/** + * @brief Return true if a temporal geo and a geo are ever within a distance + * @sqlfn eDwithin() + */ +Datum +Edwithin_tgeo_geo(PG_FUNCTION_ARGS) +{ +} + +/** + * @brief Return true if a temporal geo and a geo are always within a distance + * @sqlfn aDwithin() + */ +Datum +Adwithin_tgeo_geo(PG_FUNCTION_ARGS) +{ +} +""" + +MDB_SQL = """ +CREATE FUNCTION nearestApproachDistance(tgeompoint, stbox) + RETURNS float + AS 'MODULE_PATHNAME', 'NAD_tgeo_stbox' + LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE; +CREATE FUNCTION nearestApproachDistance(stbox, tgeompoint) + RETURNS float + AS 'MODULE_PATHNAME', 'NAD_stbox_tgeo' + LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE; +CREATE FUNCTION eDwithin(tgeompoint, geometry, float) + RETURNS boolean + AS 'MODULE_PATHNAME', 'Edwithin_tgeo_geo' + LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE; +CREATE FUNCTION aDwithin(tgeompoint, geometry, float) + RETURNS boolean + SUPPORT tspatial_supportfn + AS 'MODULE_PATHNAME', 'Adwithin_tgeo_geo' + LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE; +""" + + +def _attach(names): + idl = {"functions": [{"name": n, "api": "public"} for n in names]} + with tempfile.TemporaryDirectory() as d: + meos = Path(d) / "meos" / "src" + mdb = Path(d) / "mdb" + sql = Path(d) / "sql" + for p in (meos, mdb, sql): + p.mkdir(parents=True) + # The type-scope deriver reads MEOS's own type catalog; nothing in this + # fixture shares a wrapper, so an empty one states every fact needed. + (meos / "temporal").mkdir() + (meos / "temporal" / "meos_catalog.c").write_text("") + (meos / "x.c").write_text(MEOS_C) + (mdb / "y.c").write_text(MDB_C) + (sql / "z.sql").write_text(MDB_SQL) + idl, _, _ = attach_sqlfn_map(idl, str(meos), str(mdb), str(sql)) + return {f["name"]: f for f in idl["functions"]} + + +class EveryClaimedWrapperTests(unittest.TestCase): + + def test_a_commuted_wrapper_contributes_its_overload(self): + """The argument order a commuted wrapper registers is part of the surface.""" + f = _attach(["nad_tgeo_stbox"])["nad_tgeo_stbox"] + self.assertEqual([s["args"] for s in f["sqlSignatures"]], + [["tgeompoint", "stbox"], ["stbox", "tgeompoint"]]) + + def test_the_primary_wrapper_still_names_the_function(self): + """The union widens the signatures; it does not move `sqlfn` or `mdbC`.""" + f = _attach(["nad_tgeo_stbox"])["nad_tgeo_stbox"] + self.assertEqual(f["mdbC"], "NAD_tgeo_stbox") + self.assertEqual(f["sqlfn"], "nearestApproachDistance") + self.assertEqual(f["sqlop"], "|=|") + + def test_both_halves_of_an_ever_always_pair_are_kept(self): + """`ea_*` backs two SQL names, and each is a registration a binding emits.""" + f = _attach(["ea_dwithin_tgeo_geo"])["ea_dwithin_tgeo_geo"] + self.assertEqual([s["sqlName"] for s in f["sqlSignatures"]], + ["eDwithin", "aDwithin"]) + + def test_a_signature_from_a_second_wrapper_widens_the_arity(self): + f = _attach(["ea_dwithin_tgeo_geo"])["ea_dwithin_tgeo_geo"] + self.assertEqual((f["sqlArity"], f["sqlArityMax"]), (3, 3)) + + +class ReturnTypeTests(unittest.TestCase): + + def test_an_attribute_between_returns_and_as_is_not_part_of_the_type(self): + """PostgreSQL accepts the attributes in any order, so SUPPORT may precede + the body — `aTouches(tcbuffer, cbuffer)` is the one place MobilityDB + writes it that way.""" + rets = {name: ret for name, _, ret, _ in _create_fn_stmts(MDB_SQL)} + self.assertEqual(rets["aDwithin"], "boolean") + self.assertEqual(rets["eDwithin"], "boolean") + + def test_the_union_reports_one_return_type_when_the_wrappers_agree(self): + f = _attach(["ea_dwithin_tgeo_geo"])["ea_dwithin_tgeo_geo"] + self.assertEqual(f["sqlReturnType"], "boolean") + self.assertNotIn("sqlReturnTypeAll", f) + + +if __name__ == "__main__": + unittest.main()