Skip to content
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

chore: fallible ForeignDataWrapper trait methods #144

Merged
merged 15 commits into from
Sep 13, 2023

Conversation

imor
Copy link
Contributor

@imor imor commented Sep 12, 2023

What kind of change does this PR introduce?

Refactoring

What is the current behavior?

The ForeignDataWrapper trait's methods do not return Results even though they are fallible. This results in forcing the user to call unwrap or expect, instead of the more ergonomic alternative of returning an Err.

What is the new behavior?

The ForeignDataWrapper trait's methods will now return a Result. The error types should be convertible into an ErrorReport, i.e. they should impl From<ErrorType> for ErrorReport for the FDW's ErrorType. An ErrorReport is a pgrx type to represent errors which can be reported to Postgres. The errors returned by the trait's methods will be automatically reported to Postgres by the supabase-wrappers framework.

An example FDW with the new API looks something like this:

/// The error type has to be specified in the `error_type` field of the proc macro
#[wrappers_fdw(
    version = "0.1.0",
    author = "Supabase",
    website = "https://github.com/supabase/wrappers/tree/main/wrappers/src/fdw/helloworld_fdw",
    error_type = "HelloWorldFdwError"
)]
pub(crate) struct HelloWorldFdw {}

/// A type representing the errors returned from the ForeignDataWrapper trait's methods
enum HelloWorldFdwError {
    InvalidFoo,
    MissingBar,
}

/// A required impl for the error type to convert into ErrorReport
impl From<HelloWorldFdwError> for ErrorReport {
    fn from(value: HelloWorldFdwError) -> Self {
        let error_message = match value {
            HelloWorldFdwError::InvalidFoo => "foo is invalid",
            HelloWorldFdwError::MissingBar => "bar is missing",
        };
        // The error code and error message to return to Postgres
        ErrorReport::new(PgSqlErrorCode::ERRCODE_FDW_ERROR, error_message, "")
    }
}

impl ForeignDataWrapper<HelloWorldFdwError> for HelloWorldFdwError {
    fn new(options: &HashMap<String, String>) -> Result<Self, HelloWorldFdwError>
    where
        Self: Sized,
    {
        // Return errors from your methods, these will be automatically reported by `supabase-wrappers`
        Err(HelloWorldFdwError::InvalidFoo)
    }

    fn begin_scan(
        &mut self,
        _quals: &[Qual],
        _columns: &[Column],
        _sorts: &[Sort],
        _limit: &Option<Limit>,
        _options: &HashMap<String, String>,
    ) -> Result<(), HelloWorldFdwError> {
        // Return another error
        Err(HelloWorldFdwError::MissingBar)
    }

    fn iter_scan(&mut self, row: &mut Row) -> Result<Option<()>, HelloWorldFdwError> {
        // Return ok if no error
        Ok(None)
    }

    fn end_scan(&mut self) -> Result<(), HelloWorldFdwError> {
        // Return ok if no error
        Ok(())
    }
}

Additional context

This impl is similar to but not exactly the same as suggested in #81. The major difference is the there is no WrappersError but instead each FDW can define it's own error type. A type similar to WrappersError can be introduced later if needed.

This PR just lays the groundwork for all FDWs to better report and handle errors. But the FDWs haven't been updated with this PR. They will be done in separate, later PRs.

@imor imor marked this pull request as ready for review September 12, 2023 12:45
@imor imor requested a review from burmecia September 12, 2023 12:45
Copy link
Member

@burmecia burmecia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@burmecia burmecia merged commit 5306290 into main Sep 13, 2023
2 checks passed
@burmecia burmecia deleted the feat/fallible-fdw-trait branch September 13, 2023 06:32
@imor imor changed the title Fallible ForeignDataWrapper trait methods chore: fallible ForeignDataWrapper trait methods Oct 12, 2023
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.

2 participants