From a062f18f8b3de1a3f7fb18d6b228fa1259ca413b Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Thu, 3 Sep 2026 14:03:44 +0200 Subject: [PATCH] Read an out-array as callee-allocated only where the callee can allocate it An `outputArrays` entry says the callee allocates an array and writes its address through the parameter, so a binding passes one pointer's worth of storage and reads the array back from it. The inference admitted every non-const `**` parameter beside a by-pointer count, and eight of the twenty-two it admitted are the other contract: `json_each`, `json_each_text`, `jsonb_each`, `jsonb_each_text` and their four `pg_` twins fill an array the CALLER allocates. A binding projecting them hands MEOS eight bytes for `count` pointers and then reads the first pointer MEOS writes as the address of the array. The element type is what separates the two, and the signatures already state it. The callee writes `*p = `, an array of `E` is spelled `E *`, so a callee-allocated parameter is spelled `E **`: `TimestampTz **bins` over the by-value element `TimestampTz`, `SpanSet ***periods` over the element pointer `SpanSet *`. Stripping both levels off `Jsonb **values` leaves `Jsonb`, a value MEOS holds by reference and never by value in an array, so no array of that element exists for MEOS to have made. The predicate now strips the two levels and admits the parameter when what remains is a pointer or one of the by-value scalars the module already names for input arrays. Fourteen entries keep the classification; the eight that fill a caller's array carry none, so a binding meets them as the plain `**` argument they are. The suite states both contracts against the pair that spells the same element two ways, `jsonb_each` beside `tdwithin_tgeoarr_tgeoarr`, and the test that reads an out-array apart from an input one takes `temporal_time_split` as its subject. --- .github/workflows/pytest.yml | 2 +- parser/shapeinfer.py | 22 +++++++++++++++++++--- tests/test_shapeinfer.py | 30 +++++++++++++++++++++++++----- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 0af7100..a587bf8 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 314 + run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 315 # 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/shapeinfer.py b/parser/shapeinfer.py index aa30a32..652de4b 100644 --- a/parser/shapeinfer.py +++ b/parser/shapeinfer.py @@ -49,10 +49,26 @@ def _out_count_param(func: dict) -> str | None: def _is_written_back_array(p: dict) -> bool: - """A non-const double (or higher) pointer parameter the callee allocates - and writes back, i.e. a parallel output array.""" + """A non-const pointer parameter the callee ALLOCATES and writes back, i.e. + a parallel output array. + + The callee writes ``*p = ``, and an array of ``E`` is spelled + ``E *``, so such a parameter is spelled ``E **`` — one pointer level for the + array and one for the write-back. Stripping both leaves ``E``, and ``E`` is + what says whether the callee can have allocated the array at all: a binding + reads an array of by-value scalars (``TimestampTz **bins`` -> ``TimestampTz``) + or an array of pointers (``SpanSet ***result`` -> ``SpanSet *``), and nothing + else. A MEOS value type left bare — ``Jsonb **values`` -> ``Jsonb`` — is + neither: MEOS holds such a value by reference, never by value in an array, + so that parameter is an array the CALLER allocates and the callee only + fills. Reading it as callee-allocated makes every binding hand the callee + one element's worth of storage and take the first element it writes for the + address of the array.""" ct = p.get("cType", "") - return "**" in ct and not ct.lstrip().startswith("const") + if "**" not in ct or ct.lstrip().startswith("const"): + return False + element = _strip_one_ptr(_strip_one_ptr(ct)) + return element.endswith("*") or _bare(element) in _ELEMENT_SCALARS def _strip_one_ptr(ctype: str) -> str: diff --git a/tests/test_shapeinfer.py b/tests/test_shapeinfer.py index 00f85a4..9dcb837 100644 --- a/tests/test_shapeinfer.py +++ b/tests/test_shapeinfer.py @@ -163,17 +163,37 @@ def test_a_value_beside_a_number_is_not_an_array(self): self.assertNotIn("inputArrays", f.get("shape", {})) def test_an_out_array_is_not_read_as_an_input_one(self): - # `jsonb_each(jb, Jsonb **values, int *count)` writes `values` back, and - # its length being BY POINTER is what says so. + # `temporal_time_split(temp, ..., TimestampTz **bins, int *count)` writes + # `bins` back, and its length being BY POINTER is what says so. idl = {"functions": [_fn( - "jsonb_each", "text **", - [("jb", "const Jsonb *"), ("values", "Jsonb **"), + "temporal_time_split", "Temporal **", + [("temp", "const Temporal *"), ("bins", "TimestampTz **"), ("count", "int *")])]} idl, stats = infer_shapes(idl) shape = idl["functions"][0]["shape"] self.assertEqual(stats["inputArrays"], 0) self.assertNotIn("inputArrays", shape) - self.assertEqual(shape["outputArrays"], [{"param": "values"}]) + self.assertEqual(shape["outputArrays"], [{"param": "bins"}]) + + def test_an_array_of_bare_meos_values_is_not_callee_allocated(self): + # `jsonb_each(jb, Jsonb **values, int *count)` fills an array the CALLER + # allocates: stripping the write-back and the array levels leaves `Jsonb`, + # a value MEOS holds by reference, so `values` cannot be the address of an + # array MEOS made. Its callee-allocated sibling spells the same element + # `SpanSet ***periods`. A binding reading this one as callee-allocated + # hands MEOS one element's worth of storage for `count` of them. + idl = {"functions": [_fn( + "jsonb_each", "text **", + [("jb", "const Jsonb *"), ("values", "Jsonb **"), + ("count", "int *")]), + _fn("tdwithin_tgeoarr_tgeoarr", "int *", + [("arr1", "const Temporal **"), ("count1", "int"), + ("periods", "SpanSet ***"), ("count", "int *")])]} + idl, stats = infer_shapes(idl) + self.assertNotIn("outputArrays", idl["functions"][0]["shape"]) + self.assertEqual(idl["functions"][1]["shape"]["outputArrays"], + [{"param": "periods"}]) + self.assertEqual(stats["outputArrays"], 1) if __name__ == "__main__":