Skip to content

fix: Find VeloxType singletons for CustomType when embedded in OpaqueType objects #15829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
AshishCbMisra wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: Find VeloxType singletons for CustomType when embedded in OpaqueType objects #15829

AshishCbMisra wants to merge 2 commits into from
+232 −13

Conversation

@AshishCbMisra
Copy link

@AshishCbMisra AshishCbMisra commented Dec 19, 2025

Summary:

Background

  • This diff is to fix a bug in the Types::create() method for CustomType.

  • The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.

  • The CustomType is basically an OpaqueType object.

  • The root cause is that two different registries store OpaqueType objects.

    • Custom Type Registry → VeloxType singleton (correct name())
    • OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

  • When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.

  • The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE" in its name/toString() methods.

In this Diff

  • The primary problem is that the CustomType has a Velox singleton for its type.
    However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
  • In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

Implementation Changes

  • Types.cpp
    • Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

Testing Changes

  • OpaqueCustomTypesTest.cpp
    • Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 19, 2025
@netlify
Copy link

netlify bot commented Dec 19, 2025 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 79e4a6d
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/694b23e21cabbb0008c136b2

@meta-codesync
Copy link

meta-codesync bot commented Dec 19, 2025

@AshishCbMisra has exported this pull request. If you are a Meta employee, you can view the originating Diff in D89564769.

@AshishCbMisra AshishCbMisra changed the title Find VeloxType singletons for CustomType when embedded in OpaqueType objects Fix: Find VeloxType singletons for CustomType when embedded in OpaqueType objects Dec 20, 2025
@AshishCbMisra AshishCbMisra changed the title Fix: Find VeloxType singletons for CustomType when embedded in OpaqueType objects fix: Find VeloxType singletons for CustomType when embedded in OpaqueType objects Dec 20, 2025
@mbasmanova mbasmanova mentioned this pull request Dec 20, 2025
Open
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 22, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
24a3f15 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 22, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
4a97d0d 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 22, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
208aa00 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 22, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
2fb34e4 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 22, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
d155261 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 22, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
0187735 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
AshishCbMisra pushed a commit to AshishCbMisra/velox that referenced this pull request Dec 23, 2025
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
674a89e 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
Ashish Misra added 2 commits December 23, 2025 15:20
@facebook-github-bot
refactor: Implement one step type registration and serialization/dese…
12d2762 
…rialization for OpaqueCustomType (facebookincubator#15843)

Summary:

## Background.
- Today, a CustomType (inner type) may be wrapped by an OpaqueType (outer type), forming a OpaqueCustomType.
- When an OpaqueCustomType is defined, it needs to be registered in two place.
   - Custom Type Registry - a mapping from type name to an instance of CustomTypeFactory   that provides an instance of the type
   - OpaqueSerdeRegistry - a mapping from type name to functions that serialize and deserialize opaque type values
- Today, this requires two separate calls to the API. These separate calls are inconvenient and error prone.

## Implementation
- OpaqueCustomTypes.h
   - This PR changes the OpaqueCustomTypeRegister::registerType() function to accept optional serialization/deserialization functions, thus eliminating the need for a separate API call.
   - By keep the function arguments optional, it provides backward compatibility with existing code.
   - Implemented the VeloxType::serialize() function
   - Cleared SerializationRegistry of the serialization/deserialization functions provided in the registerType() function.

- Types.cpp
   - Implemented OpaqueType::clearSerializationRegistry() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Implements unit-tests that ensure that value serialization/deserialization functions can be provided in the registerType function optionally.
   - Implements checking for type-serialization/deserialization.

- TypesTest.cpp

Differential Revision: D89686612
@facebook-github-bot
fix: Find VeloxType singletons for CustomType when embedded in Opaque…
79e4a6d 
…Type objects (facebookincubator#15829)

Summary:

## Background
- This diff is to fix a bug in the Types::create() method for CustomType.
- The bug manifests in the SignatureBinder.bind() operation failing to match for a CustomType argument.
- The CustomType is basically an OpaqueType object. 
- The root cause is that two different registries store OpaqueType objects.
   - Custom Type Registry → VeloxType singleton (correct name())
   - OpaqueSerdeRegistry → Base OpaqueType (name() = "OPAQUE")

-  When deserializing the custom type in Type::create() method, the code does not find VeloxType singleton, hence created an OpaqueType.
-  The CustomType returns its name in its custom name in the name() and toString() methods, whereas the OpaqueType returns "OPAQUE<demangled-name>" in its name/toString() methods.

## In this Diff
- The primary problem is that the CustomType has a Velox singleton for its type.
   However, type is serialized and deserialized, the type produced is an Opaque type, with custom type inside.
- In this diff, the velox deserialization correctly finds and produces the VeloxType singleton.

## Implementation Changes
- Types.cpp
   - Implemented searching and deserializing CustomType within OpaqueTypes in Type::create() method.

## Testing Changes
- OpaqueCustomTypesTest.cpp
   - Added test to serialize/deserialize a CustomType and confirm that VeloxType singleton is returned.

Differential Revision: D89564769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@majetideepak majetideepak Awaiting requested review from majetideepak majetideepak is a code owner

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported meta-exported

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@AshishCbMisra