-
Notifications
You must be signed in to change notification settings - Fork 971
feat(duckdb): INSTALL extension
#5882
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
feat(duckdb): INSTALL extension
#5882
Conversation
2fe1a45
to
327c4ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution!
@@ -1584,6 +1584,11 @@ class Detach(Expression): | |||
arg_types = {"this": True, "exists": False} | |||
|
|||
|
|||
# https://duckdb.org/docs/sql/statements/load_and_install.html | |||
class Install(Expression): | |||
arg_types = {"this": True, "from_": False, "force": False} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just name the arg "from"
and use **kwargs
expansion to avoid hitting a reserved keyword python syntax error. We do the same for exp.Show
.
@@ -208,3 +208,53 @@ def test_token_repr(self): | |||
repr(Tokenizer().tokenize("foo")), | |||
"[<Token token_type: TokenType.VAR, text: foo, line: 1, col: 3, start: 0, end: 2, comments: []>]", | |||
) | |||
|
|||
def test_duckdb_install_tokens(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this, there's no precedence of doing this for other dialects. It's a pretty trivial code path.
# Test parsing with FROM clause | ||
install_with_from = parse_one("INSTALL spatial FROM community", dialect="duckdb") | ||
self.assertIsInstance(install_with_from, exp.Install) | ||
self.assertEqual(install_with_from.args["this"].name, "spatial") | ||
self.assertEqual(install_with_from.args["from_"].name, "community") | ||
self.assertFalse(install_with_from.args.get("force", False)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can consolidate these tests with the validate_identity
ones if we want to...
# An example
assert self.validate_identity("INSTALL httpfs").assert_is(exp.Install).name == "spatial"
@@ -1262,3 +1262,38 @@ def test_convert_datetime_time(self): | |||
|
|||
self.assertIsInstance(result, exp.TsOrDsToTime) | |||
self.assertEqual(result.sql(), "CAST('12:00:00' AS TIME)") | |||
|
|||
def test_install_expression(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one seems noisy & redundant given that we have those in DuckDB. Let's remove it or move missing cases to test_duckdb.py
Hiya 👋 |
Thanks for the work @betodealmeida , I will take it to the finish line |
Add support for parsing
INSTALL extension
in DuckDB (https://duckdb.org/docs/stable/extensions/installing_extensions.html).