Give a function the SQL surface of every wrapper it backs

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."

parser/sqlfn.py

@@ -52,6 +52,10 @@
 # binds is in the trailing `AS 'MODULE_PATHNAME', '<Wrapper>'`.
 _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 <type> 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)

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()