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: Add user-defined cell fmt interface #316

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

kysshsy
Copy link
Contributor

@kysshsy kysshsy commented Jul 31, 2024

What kind of change does this PR introduce?

feature

What is the current behavior?

The Cell type format is fixed. While developing a PR for paradeb, I discovered that types like Bytea or Timestamp might be represented differently in various databases. The fmt function of Cell currently supports only the PostgreSQL style. It would be beneficial for users to define their own formatting function while retaining the qual deparse function.

Bytea in postgres is '\x68656C6C6F' and '\x68\x65\x6C\x6C\x6F' in duckdb. It's not Compatible. etc.

What is the new behavior?

Users of the crate could implement their own custom formatting for the Cell type

Additional context

paradedb pr: paradedb/paradedb#1404

postgres bytea hex fomat
duckdb bytea hex format

@kysshsy kysshsy marked this pull request as draft July 31, 2024 13:36
@kysshsy kysshsy marked this pull request as ready for review July 31, 2024 15:41
@kysshsy
Copy link
Contributor Author

kysshsy commented Aug 2, 2024

@burmecia Hi, could you review my code or give some advice?

@burmecia
Copy link
Member

burmecia commented Aug 2, 2024

That's great, thanks for the PR! Can you merge latest main branch so the CI can run success?

@kysshsy
Copy link
Contributor Author

kysshsy commented Aug 2, 2024

That's great, thanks for the PR! Can you merge latest main branch so the CI can run success?

updated. Thank you. I am pleased to make a contribution.

@kysshsy
Copy link
Contributor Author

kysshsy commented Aug 2, 2024

seems CI fails...

@kysshsy kysshsy marked this pull request as draft August 3, 2024 02:35
@kysshsy
Copy link
Contributor Author

kysshsy commented Aug 3, 2024

This pr is a part of #320

@burmecia
Copy link
Member

burmecia commented Aug 5, 2024

seems CI fails...

yep, did some CI refactoring and should be okay now.

@kysshsy kysshsy marked this pull request as ready for review August 6, 2024 12:32
@kysshsy
Copy link
Contributor Author

kysshsy commented Aug 6, 2024

rebase and mark it as ready. :) @burmecia

@burmecia burmecia merged commit 410cbe6 into supabase:main Aug 9, 2024
3 checks passed
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