-
-
Notifications
You must be signed in to change notification settings - Fork 58
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.
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
feat(wasm): add Calendly Wasm FDW #364
Conversation
static mut INSTANCE: *mut CalendlyFdw = std::ptr::null_mut::<CalendlyFdw>(); | ||
static FDW_NAME: &str = "CalendlyFdw"; | ||
|
||
impl CalendlyFdw { | ||
fn init() { | ||
let instance = Self::default(); | ||
unsafe { | ||
INSTANCE = Box::leak(Box::new(instance)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using LazyLock<T>
instead of unsafe initialization for the INSTANCE
static.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasm doesn't have thread support and this instance is only executed in Wasm runtime, so I think the extra locking mechanism may not necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If locking is not needed then would LazyCell<T>
work? The point was to avoid using unsafe
if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of unsafe
here is to get/set the instance pointer, so we can easily to use this
(aka self
) to reference the instance. And the reason to use instance pointer is because this is a Wasm Components Model module, it will be compiled and generated binding code like this:
pub trait Guest {
/// ----------------------------------------------
/// foreign data wrapper interface functions
/// ----------------------------------------------
/// define host version requirement, e.g, "^1.2.3"
fn host_version_requirement() -> _rt::String;
/// fdw initialization
fn init(ctx: &Context) -> FdwResult;
/// data scan
fn begin_scan(ctx: &Context) -> FdwResult;
fn iter_scan(ctx: &Context, row: &Row) -> Result<Option<u32>, FdwError>;
fn re_scan(ctx: &Context) -> FdwResult;
fn end_scan(ctx: &Context) -> FdwResult;
/// data modify
fn begin_modify(ctx: &Context) -> FdwResult;
fn insert(ctx: &Context, row: &Row) -> FdwResult;
fn update(ctx: &Context, rowid: Cell, new_row: &Row) -> FdwResult;
fn delete(ctx: &Context, rowid: Cell) -> FdwResult;
fn end_modify(ctx: &Context) -> FdwResult;
}
Our fdw struct will implement that trait, but in those trait functions we cannot access self
instance, that's why I have to do a trick to store a static instance pointer as a global variable, and then deref it in the trait functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok for now @burmecia, I'll maybe try to do this in a separate PR if I find time.
What kind of change does this PR introduce?
This PR is to add a new Calendly Wasm foreign data wrapper.
What is the current behavior?
N/A
What is the new behavior?
N/A
Additional context
N/A