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

Feature: Driver - duckdb #43

Merged
merged 4 commits into from
Sep 13, 2022
Merged

Feature: Driver - duckdb #43

merged 4 commits into from
Sep 13, 2022

Conversation

oscar60310
Copy link
Contributor

@oscar60310 oscar60310 commented Sep 1, 2022

Driver for duckdb

Document:
https://github.com/Canner/vulcan/tree/feature/driver-duckdb/packages/extension-driver-duckdb

I deleted the extension for lab env because it can use this driver instead.

@oscar60310 oscar60310 force-pushed the feature/driver-duckdb branch from 75842ab to 21d7293 Compare September 1, 2022 09:23
@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2022

Codecov Report

Base: 92.59% // Head: 92.67% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (fc25b8a) compared to base (59d9e37).
Patch coverage: 97.91% of modified lines in pull request are covered.

Additional details and impacted files
@@                   Coverage Diff                    @@
##           fix/response-streams      #43      +/-   ##
========================================================
+ Coverage                 92.59%   92.67%   +0.08%     
========================================================
  Files                       222      224       +2     
  Lines                      3051     3098      +47     
  Branches                    351      358       +7     
========================================================
+ Hits                       2825     2871      +46     
- Misses                      165      166       +1     
  Partials                     61       61              
Flag Coverage Δ
build 94.87% <ø> (ø)
cli 91.90% <ø> (ø)
core 93.45% <66.66%> (-0.06%) ⬇️
extension-dbt 97.43% <ø> (ø)
extension-debug-tools 98.11% <ø> (ø)
extension-driver-duckdb 100.00% <100.00%> (?)
integration-testing 95.00% <ø> (ø)
serve 88.78% <ø> (ø)
test-utility ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/core/src/models/extensions/base.ts 86.66% <50.00%> (-5.65%) ⬇️
packages/core/src/models/extensions/dataSource.ts 100.00% <100.00%> (ø)
packages/extension-driver-duckdb/src/index.ts 100.00% <100.00%> (ø)
...xtension-driver-duckdb/src/lib/duckdbDataSource.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@oscar60310 oscar60310 force-pushed the feature/driver-duckdb branch from 21d7293 to fc25b8a Compare September 1, 2022 09:26
@oscar60310 oscar60310 changed the title [WIP] Feature: Driver - duckdb Feature: Driver - duckdb Sep 1, 2022
@oscar60310 oscar60310 requested a review from kokokuo September 1, 2022 09:28
@oscar60310 oscar60310 marked this pull request as ready for review September 1, 2022 09:28
@oscar60310 oscar60310 force-pushed the feature/driver-duckdb branch 2 times, most recently from a3ac76f to 1d639d2 Compare September 2, 2022 04:05
@oscar60310 oscar60310 linked an issue Sep 7, 2022 that may be closed by this pull request
Copy link
Contributor

@kokokuo kokokuo left a comment

Choose a reason for hiding this comment

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

Beside some parts, others LGTM 👍


# build the required packages
build: pkg-core pkg-build pkg-serve pkg-cli
build: pkg-core pkg-build pkg-serve pkg-cli pkg-serve pkg-extension-driver-duckdb
Copy link
Contributor

Choose a reason for hiding this comment

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

why execute pkg-serve twice ( after the pkg-cli ), is it a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, typo here, thanks!

*/
run(sql: string, ...params: any): void; // This last parameter should be callback function.
/**
* Convenience method for Connection#apply using a built-in default connection
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Connection#all ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks.

@oscar60310 oscar60310 force-pushed the feature/driver-duckdb branch from 1d639d2 to 7baecda Compare September 13, 2022 09:00
@oscar60310
Copy link
Contributor Author

Hi @kokokuo , all issues have been fixed.

Base automatically changed from fix/response-streams to develop September 13, 2022 10:42
@kokokuo kokokuo merged commit d28b0d8 into develop Sep 13, 2022
@kokokuo kokokuo deleted the feature/driver-duckdb branch September 13, 2022 10:42
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.

New driver: DuckDB - statement
3 participants