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

Upgrade to duckdb 1.1.3 #399

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

andrewhamon
Copy link

@andrewhamon andrewhamon commented Nov 13, 2024

Hello! I am hacking on a project using duckdb rust and noticed that the duckdb version was slightly out of date.

This came to my attention because it seems like a community plugin I am trying to use does not have updated binaries for duckdb 1.1.1 (the bigquery extension, to be specific).

Also, the upgrade script I think was missing crates/duckdb/Cargo.toml, so I added that in.

The upgrade script was missing crates/duckdb/Cargo.toml in its sed
command, causing a unit test to fail.
@andrewhamon
Copy link
Author

andrewhamon commented Nov 13, 2024

I'm a little confused about the intent of the failing test.

The panic says "expected error" but I'm wondering if thats a typo and its actually an unexpected error? edit: the panic is because a specific error was expected, but was not returned.

@andrewhamon
Copy link
Author

andrewhamon commented Nov 27, 2024

I've dug into the error a bit more. It seems like in this version of duckdb, the appender no longer errors when writing an incomplete row.

Instead, the append is ignored, and nothing seems to be written to the table (i.e. no rows are returned when querying the table).

If I can get the crate building against a local checkout of duckdb, I'll see if I can bisect when the regression was introduced.

Edit: I tripped up by not flushing. A row is written to the table, thanks to computed-column support getting added to the C api. I've updated the test to reflect this new reality.

let conn = Connection::open_in_memory()?;
conn.execute(
r"CREATE TABLE foo (
foobar TEXT,
foobar_split TEXT[] AS (split(trim(foobar), ','))
foobar_twice TEXT AS (foobar || foobar)
Copy link
Author

@andrewhamon andrewhamon Nov 27, 2024

Choose a reason for hiding this comment

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

Scalar type makes testing using query_row a bit easier (i couldn't figure out how to use query_row for a (String, Vec<String>), got some odd type errors).

Prior to duckdb/duckdb#14346, the C API did not
support appending to tables with computed columns.

This would cause panics until
duckdb#296, where a test was
introduced asserting that an append to a table with computed columns
returns an error value.

Now that appending to is supported, update the test to reflect that.
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.

1 participant