Skip to content

fix(span_table_functions): explicit unique_ptr<BinsBindData> -> unique_ptr<FunctionData> in Copy()#173

Open
estebanzimanyi wants to merge 3 commits into
MobilityDB:mainfrom
estebanzimanyi:fix/span-bins-bind-copy-unique-ptr-conv
Open

fix(span_table_functions): explicit unique_ptr<BinsBindData> -> unique_ptr<FunctionData> in Copy()#173
estebanzimanyi wants to merge 3 commits into
MobilityDB:mainfrom
estebanzimanyi:fix/span-bins-bind-copy-unique-ptr-conv

Conversation

@estebanzimanyi
Copy link
Copy Markdown
Member

GCC + DuckDB 1.4.4's unique_ptr does not implicitly convert derived to base, so return r; in BinsBindData::Copy() fails to compile:

src/temporal/span_table_functions.cpp:47:16: error: could not convert 'r'
  from 'unique_ptr<duckdb::{anonymous}::BinsBindData,...>'
  to 'unique_ptr<duckdb::FunctionData,...>'

This is a pre-existing main-branch issue that fires on a clean local build with the current vcpkg-pinned DuckDB 1.4.4. It was masked in earlier DuckDB versions which allowed the implicit derived-to-base unique_ptr conversion.

Fix

Use DuckDB's unique_ptr_cast<SRC, TGT> helper (defined in duckdb/src/include/duckdb/common/helper.hpp:130) to do the conversion explicitly:

-        return r;
+        return unique_ptr_cast<BinsBindData, FunctionData>(std::move(r));

Matches the canonical DuckDB-core pattern (see e.g. duckdb/src/include/duckdb/function/table/table_scan.hpp's TableScanBindData::Copy() which does return std::move(bind_data); for the same shape).

No behaviour change

The move is exactly what the implicit conversion would have done if the compiler accepted it. The unique_ptr_cast helper is a transparent reinterpret + ownership-transfer.

Verification

Surfaced by a fresh local build (GEN=ninja make release on a clean checkout of main + vcpkg's MEOS + DuckDB v1.4.4 submodule). After this fix, the build proceeds past span_table_functions.cpp.

…e_ptr<FunctionData> in Copy()

GCC + DuckDB 1.4.4's unique_ptr does not implicitly convert
derived->base, so 'return r;' in BinsBindData::Copy() fails to compile:

  error: could not convert 'r' from 'unique_ptr<duckdb::{anonymous}::BinsBindData,...>'
                                to 'unique_ptr<duckdb::FunctionData,...>'

Use duckdb's unique_ptr_cast helper (from duckdb/common/helper.hpp) to
do the conversion explicitly, matching the canonical pattern used by
DuckDB core (e.g. table_scan.hpp's TableScanBindData::Copy()).  No
behaviour change; the move is exactly what the implicit conversion
would have done if the compiler accepted it.
estebanzimanyi added a commit to estebanzimanyi/MobilityDuck that referenced this pull request May 21, 2026
PR MobilityDB#129's base (feat/parity-final-batch / PR MobilityDB#130) does not yet carry
the PR MobilityDB#173 fix for src/temporal/span_table_functions.cpp:47.  Cherry-
picking the th3index payload onto current MobilityDB#130 inherits the DuckDB
1.4.4 implicit derived->base unique_ptr conversion error.

This polyfill applies the same fix as MobilityDB#173 (uses unique_ptr_cast),
matching the polyfill pattern from the feat/geography-* stack.
estebanzimanyi and others added 2 commits May 21, 2026 15:49
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>
`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).
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