From 49215c1d289a03336407d40e6e5c185e70bc9913 Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Sun, 25 Oct 2026 17:41:34 +0100 Subject: [PATCH] Pass the arguments of a shorter call in the order the catalog states for the function The call leaving an argument to its NULL default drops that argument from the order the SQL arguments take, and _sql_overloads of tools/codegen_jvm.py hands _null_default_signatures that order whether the catalog states it for the signature or for the whole function, in shape.sqlArgParams. tfloatSeqSetGaps(tfloat[], maxt interval DEFAULT NULL, maxdist float DEFAULT NULL, interp text DEFAULT 'linear') reads its arguments in the order instants, maxt, maxdist, interp, which tsequenceset_make_gaps takes as instants, interp, maxt, maxdist, so tfloatSeqSetGaps(tfloat[], interval) passes its interval to maxt, -1.0 to maxdist and 'linear' to interp, as the wrapper does; raster(pixels, width, ...) without its nodata value passes its pixels last, 0.0 to nodataval and false to has_nodata. Witness: over MobilityDB master bc6f2729ea and MEOS-API master 71c23da, main's generator answers each SeqSetGaps on the typed Spark and Flink surfaces with the call stating the interval and the distance and the call stating the instants alone, and with no call stating the instants and the interval, and registers neither raster nor raquet: the shorter signature takes the order of the full one, which states arguments it does not, so it pairs nothing and is dropped. Measured over the same pair with the jar built from that catalog and that libmeos: the Spark SQL and Flink SQL surfaces each answer 1302 SQL functions and 9361 overloads where main's answer 1300 and 9349, the 12 more the call of SeqSetGaps stating the instants and the interval for its 10 temporal types, raster without its nodata value and raquet without its nodata value, and every other generated file is identical. codegen/src/test/java/JvmShorterCallOrderTest.java runs the generator over a function whose SQL arguments come in another order than its C parameters: on main the call leaving its last argument out is missing on both surfaces, and here it passes its arguments to the parameters they stand for and the default to the one it leaves out. --- .../test/java/JvmShorterCallOrderTest.java | 105 ++++++++++++++++++ tools/codegen_jvm.py | 8 +- 2 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 codegen/src/test/java/JvmShorterCallOrderTest.java diff --git a/codegen/src/test/java/JvmShorterCallOrderTest.java b/codegen/src/test/java/JvmShorterCallOrderTest.java new file mode 100644 index 000000000..547e8e387 --- /dev/null +++ b/codegen/src/test/java/JvmShorterCallOrderTest.java @@ -0,0 +1,105 @@ +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.*; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * The call leaving an argument to its NULL default, on the Flink and Spark surfaces of + * tools/codegen_jvm.py (#_sql_overloads there), when the catalog states the order of the SQL + * arguments for the whole function, in shape.sqlArgParams, rather than for the signature: the + * shorter call drops the argument it leaves out from that order, and passes its arguments to the + * C parameters they stand for, as raster(pixels, width, ...) passes its pixels to the last C + * parameter whether or not it states a nodata value. The generator runs over a catalog of one + * function, as JvmStructRowTest runs it, so no jar and no libmeos. + */ +class JvmShorterCallOrderTest { + + private static final Path TOOLS = Path.of("..", "tools").toAbsolutePath().normalize(); + + /** gaps(a int, b int, c double) in C, read by the SQL signature + * gaps(c float, a integer, b integer DEFAULT NULL), whose wrapper passes 0 for a NULL b. */ + private static final String CATALOG = "{\"functions\": [" + + "{\"name\": \"gaps\", \"file\": \"meos.h\", \"api\": \"public\", \"sqlfn\": \"gaps\"," + + " \"returnType\": {\"c\": \"int64_t\", \"canonical\": \"int64_t\"}," + + " \"params\": [{\"name\": \"a\", \"cType\": \"int\", \"canonical\": \"int\"}," + + " {\"name\": \"b\", \"cType\": \"int\", \"canonical\": \"int\"}," + + " {\"name\": \"c\", \"cType\": \"double\", \"canonical\": \"double\"}]," + + " \"shape\": {\"sqlArgParams\": [\"c\", \"a\", \"b\"]}," + + " \"sqlSignatures\": [{\"args\": [\"float\", \"integer\", \"integer\"], \"ret\": \"bigint\"," + + " \"argDefaults\": [null, null, \"NULL\"], \"nullDefaultBinds\": {\"2\": {\"b\": \"0\"}}}]}]," + + " \"typeEncodings\": {}, \"structs\": []," + + " \"objectModel\": {\"classes\": {}}, \"enums\": [], \"macros\": []}"; + + private static final String JAR = "{" + + "\"gaps\": {\"ret\": \"long\", \"arg_types\": [\"int\", \"int\", \"double\"]}}"; + + /** Prints {engine: [{args, call}]}, one entry per overload of gaps the engine registers. */ + private static final String DRIVER = String.join("\n", + "import json, sys", + "sys.path.insert(0, sys.argv[1])", + "import codegen_jvm as g", + "from collections import defaultdict", + "cat = json.load(open(sys.argv[2]))", + "jar = json.load(open(sys.argv[3]))", + "out = {}", + "for engine, m in (('flink', g.SqlModel(cat, jar)),", + " ('spark', g.SqlModel(cat, jar, g.SPARK_SQL_PKG, 'spark'))):", + " rows = []", + " for name, sargs, sdef, ov, why, isset, arity in g._sql_overloads(", + " m, cat, jar, m.layout, defaultdict(int), lambda f: True):", + " if ov is not None:", + " rows.append({'args': sargs, 'call': [l for l in ov[3] if 'gaps(' in l]})", + " out[engine] = rows", + "print(json.dumps(out))", + ""); + + private static JsonNode results; + + @BeforeAll + static void generate() throws Exception { + Path dir = Files.createTempDirectory("shorter-call"); + try { + Files.writeString(dir.resolve("catalog.json"), CATALOG); + Files.writeString(dir.resolve("jar.json"), JAR); + Files.writeString(dir.resolve("driver.py"), DRIVER); + Process p = new ProcessBuilder("python3", "-B", dir.resolve("driver.py").toString(), + TOOLS.toString(), dir.resolve("catalog.json").toString(), + dir.resolve("jar.json").toString()).redirectErrorStream(true).start(); + String out = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertTrue(p.waitFor(120, TimeUnit.SECONDS), "the generator ran"); + assertEquals(0, p.exitValue(), out); + results = new ObjectMapper().readTree(out); + } finally { + try (var stream = Files.walk(dir)) { + stream.sorted(java.util.Comparator.reverseOrder()).forEach(q -> q.toFile().delete()); + } + } + } + + @Test + void theCallStatingEveryArgumentPassesThemInTheirCOrder() { + for (String engine : new String[] {"flink", "spark"}) { + JsonNode full = results.get(engine).get(0); + assertEquals("[\"float\",\"integer\",\"integer\"]", full.get("args").toString(), engine); + assertTrue(full.get("call").toString().contains("GeneratedFunctions.gaps(a1, a2, a0)"), + engine + " " + full); + } + } + + @Test + void theCallLeavingAnArgumentOutPassesWhatTheWrapperPassesForIt() { + for (String engine : new String[] {"flink", "spark"}) { + assertEquals(2, results.get(engine).size(), engine + " " + results.get(engine)); + JsonNode shorter = results.get(engine).get(1); + assertEquals("[\"float\",\"integer\"]", shorter.get("args").toString(), engine); + assertTrue(shorter.get("call").toString().contains("GeneratedFunctions.gaps(a1, 0, a0)"), + engine + " " + shorter); + } + } +} diff --git a/tools/codegen_jvm.py b/tools/codegen_jvm.py index 7788eb965..e8a7e415f 100644 --- a/tools/codegen_jvm.py +++ b/tools/codegen_jvm.py @@ -1457,8 +1457,12 @@ def _sql_overloads(m, cat, jmeos, layout, skipped, keep): f.get('sqlSignatures') or []): ov, why, isset = _signature_overload(m, f, sig, sargs, sret, jsig, sbound, layout) yield sqlname, sargs, sdef, ov, why, isset, None - for arity, dsig, dargs, ddef, dbound in _null_default_signatures(sig, sargs, sdef, - sbound): + # The shorter calls drop the left-out arguments from the order the SQL arguments + # take, which the catalog states for the signature or for the whole function + # (`raster(pixels, width, ...)` reads its pixels first) + order = sig.get('sqlArgParams') or (f.get('shape') or {}).get('sqlArgParams') + for arity, dsig, dargs, ddef, dbound in _null_default_signatures( + dict(sig, sqlArgParams=order) if order else sig, sargs, sdef, sbound): dov, _, disset = _signature_overload(m, f, dsig, dargs, sret, jsig, dbound, layout) if dov is not None: