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

absolute DuckDB FileAttachment URL #389

Merged
merged 4 commits into from
Nov 29, 2023
Merged

absolute DuckDB FileAttachment URL #389

merged 4 commits into from
Nov 29, 2023

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Nov 18, 2023

I've only tested this locally (not sure how unit tests could work).

src/duckdb.js Outdated Show resolved Hide resolved
src/duckdb.js Outdated
@@ -208,7 +208,7 @@ async function insertFile(database, name, file, options) {
}
if (/\.parquet$/i.test(file.name)) {
return await connection.query(
`CREATE VIEW '${name}' AS SELECT * FROM parquet_scan('${file.name}')`
`CREATE VIEW '${name}' AS FROM parquet_scan('${file.name}')`
Copy link
Member

Choose a reason for hiding this comment

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

Is the shorter form recommended? Seems like the docs still use SELECT *.

https://duckdb.org/docs/data/parquet/overview.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's brand new. Documented in https://duckdb.org/docs/sql/query_syntax/from with one sentence “DuckDB also has an optional FROM-first syntax which enables you to also query without a SELECT statement.” The announcement (August 2023) mentions that SELECT * is "obsolete".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll revert that commit for now, since I'm not sure when the new syntax was introduced and it's not related to the FileAttachment issue.

Fil and others added 2 commits November 26, 2023 23:32
Co-authored-by: Mike Bostock <[email protected]>
This reverts commit 9d88421.
@Fil Fil marked this pull request as ready for review November 27, 2023 07:55
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Re-approving. 😁

@Fil
Copy link
Contributor Author

Fil commented Nov 29, 2023

I feel compelled to click on the ✅

@Fil Fil merged commit 7f41029 into main Nov 29, 2023
3 checks passed
@Fil Fil deleted the fil/absolute-parquet branch November 29, 2023 16:37
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.

2 participants