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/pg name support #504

Merged
merged 5 commits into from
Jul 3, 2023
Merged

Conversation

Jordan-M-Young
Copy link
Contributor

The goal of this PR is to extend support for the Postgres name type requested within the issue: #501. This issue centers around connectorx panicking when interacting with tables containing a name type.

To solve this, I added:

a Name type in the various enums in connectorx/src/sources/postgres/typesystem.rs
a mapping between Name and LargeUtf8 in connectorx/src/transports/postgres_arrow2.rs
added NAME data to scripts/postgres.sql
added a test for NAME compatibility with postgres/polars in connectorx/tests/test_polars.rs

@Jordan-M-Young
Copy link
Contributor Author

@wangxiaoying hope you're well! Would you mind taking a look at this when you have a chance? Thanks!

@wangxiaoying
Copy link
Contributor

wangxiaoying commented Jun 16, 2023

Hi @Jordan-M-Young , thanks for the PR. It looks good to me for arrow2/polars.

Could you also add this support for arrow and pandas, basically the transport mapping in connectorx/src/transports/postgres_arrow.rs and connectorx-python/src/pandas/transports/postgres.rs, as well their corresponding tests? (Looks like #475 is using pandas for the result type)

After this we can merge. Thanks!

@MatsMoll
Copy link
Contributor

Any idea when this will get merged? 😄

@Jordan-M-Young
Copy link
Contributor Author

Any idea when this will get merged? 😄

Hey @MatsMoll, Things have been busy with my job lately but I should have some time this weekend to finish up the PR!

@Jordan-M-Young
Copy link
Contributor Author

Hey @wangxiaoying I think my latest commit satisfies your requests above. If so would you mind merging?

@wangxiaoying
Copy link
Contributor

Thanks @Jordan-M-Young !

@wangxiaoying wangxiaoying merged commit f1d030e into sfu-db:main Jul 3, 2023
@Jordan-M-Young
Copy link
Contributor Author

@wangxiaoying Thank you as always! @MatsMoll shouldn't be long before this is released!

@wangxiaoying
Copy link
Contributor

0.3.2-alpha.7 will be released when this is done!

@MatsMoll
Copy link
Contributor

MatsMoll commented Jul 8, 2023

Thank you for merging this! Appreciate it @Jordan-M-Young @wangxiaoying

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.

3 participants