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

feat(wasm): add Clerk Wasm FDW #404

Merged
merged 10 commits into from
Feb 5, 2025
Merged

feat(wasm): add Clerk Wasm FDW #404

merged 10 commits into from
Feb 5, 2025

Conversation

burmecia
Copy link
Member

What kind of change does this PR introduce?

This PR is to add a new Clerk Wasm foreign data wrapper.

What is the current behavior?

N/A

What is the new behavior?

N/A

Additional context

N/A

@burmecia burmecia added the wasm label Jan 24, 2025
@burmecia burmecia requested review from olirice and imor January 24, 2025 09:05
@burmecia burmecia marked this pull request as draft January 25, 2025 00:29
@burmecia burmecia marked this pull request as ready for review January 25, 2025 05:06
@burmecia
Copy link
Member Author

CI failed due to pgcentralfoundation/pgrx#1966

@burmecia
Copy link
Member Author

burmecia commented Feb 3, 2025

Need to wait for #409 to be merged first to fix CI issue.

@colinclerk
Copy link

Hello - thank you for putting this together! I read through the PR and we're going to suggest a few adjustments (change table name to clerk_{object}, and cherry pick a more complete column set, e.g. I know we'd like to get primary_email_address in there directly instead of having to loop through attrs).

Would you prefer we fork this branch or another process? Since we'd plan on changing the table names, everything we suggest will be backward compatible, so there's no need to block on us if you want to move forward.

Thanks again!

@burmecia
Copy link
Member Author

burmecia commented Feb 5, 2025

Thanks for your suggestions @colinclerk.

We already suggested using dedicated schema clerk to save foreign tables, so I don't think table name prefix like clerk_{object} is necessary, but that's all up to the user, they can use whatever names they like and we just provide recommendations to better organise the tables.

For cheery picking column set, we definitely can pick some more properties if you think they are more important, but that only works for 1-to-1 properties. For some properties like email addresses, they are 1-to-many relationship with the object, and currently the foreign table can only return 1 row for 1 object, so we'd better to keep those properties in json column and let user use subquery or view to extract it out.

You can directly push on this branch or create another fork, we'd like to see more feedbacks from you for this fdw and thanks again!

@burmecia burmecia merged commit 8757659 into main Feb 5, 2025
6 checks passed
@burmecia burmecia deleted the bo/feat/clerk-fdw branch February 5, 2025 10:53
@wobsoriano
Copy link

wobsoriano commented Feb 6, 2025

Hi @burmecia, I saw the recent release that includes this update and immediately tried it out. It works perfectly! Thank you for implementing this 🫡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants