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.

Sign up for GitHub

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

Jump to bottom

vdk-duckdb: fix ingestion #2843

Merged
merged 1 commit into from
Oct 31, 2023
Merged

vdk-duckdb: fix ingestion #2843

merged 1 commit into from
Oct 31, 2023

Conversation

antoniivanov
Copy link
Collaborator

@antoniivanov antoniivanov commented Oct 27, 2023 •
edited
Loading

Ingestion was not working properly due to multiple issues so it's being fixed.

  • The plugin now seamlessly integrates with the VDK connection interface, making it easier to manage DuckDB connections instead of relying on custom DuckDB Connection
  • The ingestion insertion mechanism fixes:
    • The ingestion method was not registered (so job_input.send(method='duckdb') was not working)
    • Change the insertion to be batch and not one by one (which is very slow)
    • added ingestion tests to cover regressions and verify it's working
  • Renamed duckdb_file to database (which is what duckdb name for that parameter is)

@antoniivanov antoniivanov force-pushed the person/aivanov/duckdb branch 2 times, most recently from 4930798 to e361393 Compare October 27, 2023 07:17
@mrMoZ1
Copy link
Contributor

mrMoZ1 commented Oct 27, 2023

Could you update the PR description. Right now it's a bit vague.

@antoniivanov
vdk-duckdb: fix ingestion
57bd490 
Ingestino was not working properly due to multiple issues so it's being
fixed.

Signed-off-by: Antoni Ivanov <[email protected]>
@antoniivanov antoniivanov force-pushed the person/aivanov/duckdb branch from e361393 to 57bd490 Compare October 30, 2023 10:58
mrMoZ1
mrMoZ1 approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@mrMoZ1 mrMoZ1 left a comment

Choose a reason for hiding this comment

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

LGTM

@antoniivanov antoniivanov merged commit 6c4a89f into main Oct 31, 2023
@antoniivanov antoniivanov deleted the person/aivanov/duckdb branch October 31, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mrMoZ1 mrMoZ1 mrMoZ1 approved these changes

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@antoniivanov @mrMoZ1 @vmwclabot