Skip to content

Conversation

@gpsamson
Copy link
Contributor

@gpsamson gpsamson commented Mar 8, 2023

This pull request fixes an issue where enum array columns would fail with because of an empty namespace value:

07:23:45     ERROR:  syntax error at or near "."
07:23:45    LINE 1: SELECT unnest(enum_range(NULL::.my_enum_type))

For some reason, there is a condition that throws an error if an array value's length doesn't match its typlen value. This doesn't make sense since Postgres arrays are variable length. Existing array tests seem to pass without it.

Testing

I added some additional unit tests for enum arrays. Github Actions was slow to start so pasting test output from my machine:

[100%] Built target postgres_scanner_loadable_extension
./build/release/test/unittest --test-dir . "[postgres_scanner]"
Filters: [postgres_scanner]
[17/17] (100%): test/postgres_scanner/bug76.test
===============================================================================
All tests passed (30776 assertions in 17 test cases)

@gpsamson
Copy link
Contributor Author

gpsamson commented Mar 8, 2023

@Mytherin Can't request you as a reviewer, no rush to get this reviewed.

Once merged in, it looks like I need to open a PR to bump the extensions CSV here: https://github.com/duckdb/duckdb/blob/master/.github/config/extensions.csv#L12. Lmk if not. Thanks again

@Mytherin
Copy link
Contributor

Mytherin commented Mar 8, 2023

Could we add a test file that reads from the enum tables/column as well?

@Mytherin
Copy link
Contributor

Mytherin commented Mar 8, 2023

As far as updating the extensions in the main repo - that is indeed required to update the extension for the next release, but it won’t update the extension for the current release so there is no immediate rush.

@jhrcek
Copy link

jhrcek commented Mar 9, 2023

@gpsamson awesome, thanks for working on this!
I built and tested your PR locally and I'm now able to run postgres_attach without errors.

@Mytherin Mytherin merged commit c3bfcec into duckdb:main Mar 9, 2023
@Mytherin
Copy link
Contributor

Mytherin commented Mar 9, 2023

Thanks!

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