Skip to content

fix: drop broken using meosType = MeosType; alias in tydef.hpp#161

Open
estebanzimanyi wants to merge 4 commits into
MobilityDB:mainfrom
estebanzimanyi:fix/tydef-meostype-alias
Open

fix: drop broken using meosType = MeosType; alias in tydef.hpp#161
estebanzimanyi wants to merge 4 commits into
MobilityDB:mainfrom
estebanzimanyi:fix/tydef-meostype-alias

Conversation

@estebanzimanyi
Copy link
Copy Markdown
Member

High-leverage 1-file fix unblocking every in-flight PR's Linux arm64 build.

The bug

src/include/tydef.hpp:18 reads:

using meosType = MeosType;

This was added in c8cad6d ("Pre-existing build issue surfaced when the headers were touched — vcpkg's MEOS now exposes MeosType instead of meosType…") on the assumption that the pinned MEOS exposed the upper-case spelling. That assumption is no longer true.

The pinned MEOS (vcpkg_ports/meos/portfile.cmake REF f11b7443ee98…) exposes meosType (lower-case) via meos/include/temporal/meos_catalog.h:

extern uint32 datum_hash(Datum d, meosType basetype);
extern Set *set_in(const char *str, meosType basetype);
// …

So the alias resolves using meosType = MeosType; where MeosType doesn't exist → every PR's Linux arm64 build dies with:

/duckdb_build_dir/src/include/tydef.hpp:18:18:
  error: ‘MeosType’ does not name a type; did you mean ‘meosType’?

Confirmation that MobilityDuck doesn't actually need MeosType

$ grep -rn '\bMeosType\b' src/ --include='*.cpp' --include='*.hpp'
src/include/tydef.hpp:14:// Forward-compat alias for the meosType → MeosType rename…
src/include/tydef.hpp:15:// pr785-sync-script).  Vcpkg's MEOS exposes \`MeosType\`; existing
src/include/tydef.hpp:18:using meosType = MeosType;     # ← the broken line

MeosType is referenced only by the alias line and its comment. Every other call-site uses the lower-case meosType (which MEOS already provides):

$ grep -rn '\bmeosType\b' src/ --include='*.cpp' --include='*.hpp' | wc -l
30+   # ≥ 30 active call-sites, all in src/temporal/spanset_functions.cpp and similar

The fix

Delete the alias line and replace its comment with one that explains the current state (so future MEOS-pin bumps don't re-introduce the same bug). Zero behavioural change to MobilityDuck.

Impact: unblocks the whole open-PR backlog

Every Linux arm64 build currently fails identically — this includes:

PR Title
#126 parity batch — bearing + eCovers/tCovers + stbox dim + seqSetGaps
#130 feat(parity): close final 14-name gap to reach 100% addressable coverage
#149 Longjmp out of the MEOS error handler
#158 edge-to-cloud quickstart + temporalFooter + SRID/geodetic fix
#159 doc: PR Reviewer Guide (rebased #117)
#160 docs+scripts: PR coordination policy (rebased #120)
#148 / #150 / #151 / #153 / #155 / #156 the feat/*_port_core extended-type stack

…all of which would build green once this merges.

Refs

Unblocks #126, #130, #149, #158, #159, #160 + the *_port_core stack.

`meosType` (lower-case) is the **pre-consolidation** MEOS type name;
`MeosType` (upper-case) is the **post-consolidation** target that the
upstream rename sweep has not yet reached.  The current vcpkg pin
(`vcpkg_ports/meos/portfile.cmake` REF f11b7443ee98…) is still
pre-consolidation: `meos/include/temporal/meos_catalog.h` line 121
declares the typedef as `} meosType;` and every MEOS API uses the
lower-case spelling.  MobilityDuck's source code consistently uses
`meosType` to match — `grep -rn '\bMeosType\b' src/` finds the name
only on the alias line and its comment, nowhere else.

c8cad6d added `using meosType = MeosType;` as a forward-looking
bridge for the eventual consolidation bump.  That bridge points at
`MeosType`, which the current pin does NOT yet expose, so it
breaks every PR's Linux arm64 build with:

  /duckdb_build_dir/src/include/tydef.hpp:18:18:
    error: ‘MeosType’ does not name a type; did you mean ‘meosType’?

The fix is to drop the premature alias and replace the misleading
comment with one that documents the pre/post-consolidation distinction
and the resume path for the next pin bump — at that point a reviewer
can either restore the bridge (this time it'll be valid because
`MeosType` will exist) or sweep the MobilityDuck source from
`meosType` to `MeosType` in a single PR.

Unblocks every in-flight PR's Linux arm64 build: MobilityDB#126, MobilityDB#130, MobilityDB#149,
MobilityDB#158, MobilityDB#159, MobilityDB#160, plus the entire `feat/*_port_core` extended-type
stack (MobilityDB#148/MobilityDB#150/MobilityDB#151/MobilityDB#153/MobilityDB#155/MobilityDB#156).
@estebanzimanyi estebanzimanyi force-pushed the fix/tydef-meostype-alias branch from 4119a7d to cf36886 Compare May 20, 2026 10:09
@estebanzimanyi
Copy link
Copy Markdown
Member Author

Reviewer's quickstart — ~2 minutes

What this PR does in one sentence: removes one line (using meosType = MeosType;) from src/include/tydef.hpp that was added on the assumption that the vcpkg MEOS pin had been bumped past the consolidation rename — but the current pin still exposes the pre-consolidation meosType, so the alias references a non-existent type and breaks every PR's Linux arm64 build.

Files to read (1 file, 1 line removed):

  • src/include/tydef.hpp — drop line 18 + update the comment above it to document the pre/post-consolidation distinction so the next pin bump knows what to do.

Naming context (for clarity, not action):

  • meosType (lowercase) = pre-consolidation spelling, exposed by the current pin f11b7443ee98…
  • MeosType (uppercase) = post-consolidation target (the rename is upstream-MobilityDB work in flight; not yet reflected in the pin)
  • MobilityDuck's source consistently uses meosType (matches the pin) — grep -rn '\bMeosType\b' src/ confirms MeosType is referenced ONLY on the alias line itself.

Smoke verification (Linux arm64 CI): already SUCCESS on this PR's most recent run. Linux amd64 FAILURE was a transient curl network error in vcpkg, not a code issue — re-trigger to confirm.

Why it's safe to merge: no-op for MobilityDuck's own code (which uses meosType directly); unblocks ~10 in-flight PRs (#126, #130, #149, #158, #159, #160 + the feat/*_port_core stack) that all fail the same 'MeosType' does not name a type build step on Linux arm64.

When the MEOS pin is bumped past consolidation (future PR): either restore the bridge (using meosType = MeosType; will then be valid because MeosType will exist) or sweep MobilityDuck source meosType → MeosType in one PR — the comment in the modified file documents both options.

This was referenced May 20, 2026
…MobilityDB#136)

Cherry-picked from open PR MobilityDB#136 (commit 9e1d7a6) so this PR's CI goes
green before MobilityDB#136 lands. When MobilityDB#136 reaches main, the rebase will
collapse this commit to a no-op and it will drop out.

--- original commit body ---
Pre-stage icu extension for amd64 docker tests

LoadInternal calls ExtensionHelper::AutoLoadExtension(db, "icu") so the
Europe/Brussels timezone option is honoured. Inside the linux_amd64 test
docker container there is no network egress and the local extension
directory is empty, so the autoload fails. Copy the icu.duckdb_extension
that was just built locally (declared in extension_config.cmake) into the
expected path before running the unittester.
…en PR MobilityDB#140)

On macOS LP64 and Wasm/emscripten, int64 (long) and int64_t (long long) are
the same width but distinct types, so clang rejects passing bigint_to_set
where a Set *(*)(int64_t) is expected as a non-type template arg. Cherry-
picked from open PR MobilityDB#140 (a8b1755) so this PR goes green on osx_amd64,
osx_arm64, and wasm_mvp before MobilityDB#140 lands. The cast is a no-op on Linux,
where int64 and int64_t are both long. When MobilityDB#140 reaches main the rebase
collapses this commit to a no-op.
estebanzimanyi added a commit to estebanzimanyi/MobilityDuck that referenced this pull request May 20, 2026
… PR MobilityDB#161)

Cherry-picked from open PR MobilityDB#161 so this PR's CI compiles against the
vcpkg-installed MEOS, which exposes 'meosType' (pre-consolidation)
not 'MeosType'.  When MobilityDB#161 reaches main, this commit collapses to a
no-op on rebase.
estebanzimanyi added a commit to estebanzimanyi/MobilityDuck that referenced this pull request May 21, 2026
… PR MobilityDB#161)

Cherry-picked from open PR MobilityDB#161 so this PR's CI compiles against the
vcpkg-installed MEOS, which exposes 'meosType' (pre-consolidation)
not 'MeosType'.  When MobilityDB#161 reaches main, this commit collapses to a
no-op on rebase.
estebanzimanyi added a commit to estebanzimanyi/MobilityDuck that referenced this pull request May 21, 2026
… PR MobilityDB#161)

Cherry-picked from open PR MobilityDB#161 so this PR's CI compiles against the
vcpkg-installed MEOS, which exposes 'meosType' (pre-consolidation)
not 'MeosType'.  When MobilityDB#161 reaches main, this commit collapses to a
no-op on rebase.
estebanzimanyi added a commit to estebanzimanyi/MobilityDuck that referenced this pull request May 21, 2026
… PR MobilityDB#161)

Cherry-picked from open PR MobilityDB#161 so this PR's CI compiles against the
vcpkg-installed MEOS, which exposes 'meosType' (pre-consolidation)
not 'MeosType'.  When MobilityDB#161 reaches main, this commit collapses to a
no-op on rebase.
The stage_icu helper mapped only the Linux uname values, so on the
macOS arm64 test runner uname -m returned "arm64" and the icu
extension was copied to .duckdb/extensions/v1.4.4/arm64 instead of
.../osx_arm64, where DuckDB's autoload looks. The hub fallback is not
reliably resolvable on that runner, so the osx_arm64 Test step failed
to load the extension. Map the OS and architecture to the DuckDB
platform string (linux_amd64, linux_arm64, osx_amd64, osx_arm64) so
the locally built icu is staged at the path autoload expects on every
tested platform; the Linux mapping is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant