Skip to content

Conversation

taniabogatsch
Copy link
Collaborator

@taniabogatsch taniabogatsch commented Sep 24, 2025

While working on the QueryAppender in #545, I noticed some inconsistencies in value.go. This PR refactors the file to be more consistent with duckdb terminology. It also prepares for changes like inferring the appender's temporary table types from the arguments passed to AppendRow.

@taniabogatsch taniabogatsch added the feature / enhancement Code improvements or a new feature label Sep 24, 2025
Copy link
Collaborator

@mlafeldt mlafeldt left a comment

Choose a reason for hiding this comment

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

Those new names are much better. 👍

I only have a comment regarding testing.

@taniabogatsch taniabogatsch marked this pull request as draft September 25, 2025 10:22
Copy link
Collaborator

@mlafeldt mlafeldt left a comment

Choose a reason for hiding this comment

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

LGTM overall. Just some nits left.

@mlafeldt mlafeldt marked this pull request as ready for review September 26, 2025 16:08
@taniabogatsch
Copy link
Collaborator Author

I'll open a follow-up PR to add the missing types for #545 - I think these are mainly structs (in the form of map[string]any) and maps (if passed as Map).

@taniabogatsch taniabogatsch merged commit 6c6ebd0 into marcboeker:main Sep 28, 2025
17 checks passed
@taniabogatsch taniabogatsch deleted the value-refactor branch September 28, 2025 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature / enhancement Code improvements or a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants