Attach the @sqlfn SQL-name map to the catalog#18
Open
estebanzimanyi wants to merge 4 commits into
Open
Conversation
estebanzimanyi
force-pushed
the
feat/sqlfn-name-map
branch
from
a415613 to
19890c3
Compare
Follow each function's @csqlfn -> wrapper @sqlfn -> SQL name chain and attach the resulting SQL name to the catalog. The vendored-source root is overridable via MDB_SRC_ROOT so the @sqlfn (and @InGroup) extraction can be pointed at the same pinned MobilityDB checkout as the headers, keeping the generated catalog reproducibly equivalent to that pin.
estebanzimanyi
force-pushed
the
feat/sqlfn-name-map
branch
from
19890c3 to
a8779b4
Compare
A copy-paste @csqlfn in meos/src can point an ever/always spatial-relationship function (named <e|a><verb>_...) at the opposite-prefix MobilityDB-C wrapper — e.g. eintersects_tgeo_geo is tagged `@csqlfn #Aintersects_tgeo_geo`, so the chain resolves its @sqlfn to aIntersects instead of eIntersects. That silently drops the real (tgeo,geo) overload from the eIntersects group in a binding's overload dispatcher, leaving only a wrong-subtype backing reachable — which then raises a runtime "The temporal value must be of type tcbuffer" on a tgeompoint (observed in the MobilitySpark BerlinMOD bench, q17). The parser is faithful — it propagates whatever the source tags say — so this is a source defect to fix in meos/src, not a parser bug. Add lint_ea_sqlfn() and report the mismatches at catalog-gen so they surface loudly instead of shipping a wrong @sqlfn silently. Flags 5 live mistags (eintersects_tgeo_geo, etouches_tpoint_geo/_tcbuffer_geo/_tcbuffer_cbuffer, econtains_geo_trgeo); relayed to the source maintainers to correct the @csqlfn tags.
A single MEOS function can back more than one MobilityDB-C wrapper: the ever/always pair (eDisjoint/aDisjoint) shares one ea_* function, and the shift/scale/shift_scale trio shares one C function. Such functions carry one @csqlfn with comma- or space-separated #Wrapper() references that may continue across doxygen lines. _meos_to_mdb reads the whole tag value and returns every wrapper, and attach_sqlfn_map records all the SQL names a function exposes (sqlfn/sqlfnAll, mdbC/mdbCAll).
Add lint_sqlfn_case_collisions next to lint_ea_sqlfn: it groups every @sqlfn (and sqlfnAll) name by its lower-case form and flags any group with more than one spelling (e.g. tDistance vs tdistance), wired into run.py as a loud warning. Such names are the SAME SQL function — PostgreSQL folds unquoted identifiers to lower case — so the clash is invisible in SQL and pg_regress. But the binding name is case-sensitive, and case-insensitive engines (Spark SQL) register every spelling under one UDF, so one silently shadows the other (e.g. the trgeometry tdistance shadowing the tgeo tDistance dispatch, returning NULL). Surfacing it at catalog generation lets the casing straggler be fixed at the MEOS-C @sqlfn source before it reaches any binding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The catalog carries MEOS-C names + C signatures but not the user-facing SQL names that SQL/UDF-emitting bindings (MobilitySpark, MobilityDuck, MobilityDB SQL) need. Both are machine-extractable from the doxygen tag chain already in the source:
parser/sqlfn.pyextracts and joins the chain from the vendored_mobilitydb/source;run.pyattachessqlfn/sqlop/mdbCper function as step 4. 1997 / 2830 functions now carry their SQL name (e.g.tpoint_length->length,econtains_geo_tgeo->eContains,temporal_value_at_timestamptz->valueAtTimestamp); the rest are internal (no SQL surface).Binding generators can now read SQL names from the single SoT instead of per-binding hand-maps.
Byproduct — wrappers missing
@sqlfn(chain reachable but tag absent), for a follow-up source pass:Stbox_constructor(->stbox),Temporal_after_timestamptz(->afterTimestamp),Temporal_before_timestamptz(->beforeTimestamp),Temporal_simplify_max_dist(->maxDistSimplify).