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

Runtime: duckdb sqlite extensions based connector for sqlite #3018

Merged
merged 14 commits into from
Sep 26, 2023

Conversation

k-anshul
Copy link
Member

@k-anshul k-anshul commented Sep 4, 2023

  • Adds a sqlite connector

Copy link
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

Hey, I took a look at this PR and added some comments in Notion here

@k-anshul k-anshul changed the title sqlite and postgres extensions based connectors Runtime: duckdb sqlite extensions based connector for sqlite Sep 8, 2023
@k-anshul k-anshul marked this pull request as ready for review September 8, 2023 11:31
Copy link
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

Can you also merge main and update for changed driver interfaces?

runtime/drivers/sqlite/sqlite.go Outdated Show resolved Hide resolved
@k-anshul k-anshul marked this pull request as draft September 15, 2023 12:07
@k-anshul k-anshul marked this pull request as ready for review September 25, 2023 10:38
runtime/drivers/sqlite/sqlite.go Outdated Show resolved Hide resolved
@@ -175,7 +175,7 @@ func (a *AST) traverseTableFunction(parent astNode, childKey string) {
case "read_csv_auto", "read_csv",
"read_parquet",
"read_json", "read_json_auto", "read_json_objects", "read_json_objects_auto",
"read_ndjson_objects", "read_ndjson", "read_ndjson_auto":
"read_ndjson_objects", "read_ndjson", "read_ndjson_auto", "sqlite_scan":
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems weird – the value passed to sqlite_scan isn't really a path. Should we handle it as a separate case? And also check the callers to duckdbsql that they can handle sqlite table functions (they might assume it's a file right now).

runtime/services/catalog/migrator/sources/sources.go Outdated Show resolved Hide resolved
web-common/src/features/sources/sourceUtils.ts Outdated Show resolved Hide resolved
Co-authored-by: Benjamin Egelund-Müller <[email protected]>
@begelundmuller begelundmuller added the blocker A release blocker issue that should be resolved before a new release label Sep 25, 2023
@begelundmuller begelundmuller merged commit cc4dcc4 into main Sep 26, 2023
4 checks passed
@begelundmuller begelundmuller deleted the sql_extensions branch September 26, 2023 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker A release blocker issue that should be resolved before a new release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants