Use duckdb functions without having to specify the full type signature #531
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JelteF
force-pushed
the
redesigned-read-parquet
branch
5 times, most recently
from
ffb9a26 to
59d8a57
Compare
JelteF
force-pushed
the
redesigned-read-parquet
branch
from
59d8a57 to
1992351
Compare
JelteF
commented
Jan 14, 2025
src/pgduckdb_options.cpp
Outdated
| } | ||
|
|
||
| if (list_length(indirection) != 1) { | ||
| /* TODO: Fix this, if the column contains an array we should be able to subscript that type */ |
There was a problem hiding this comment.
This needs a test with an array/json column.
JelteF
commented
Jan 14, 2025
Comment on lines
+422
to
+423
| /* TODO: Trigger cache population, probably we should do this somewhere else */ | ||
| pgduckdb::IsExtensionRegistered(); |
There was a problem hiding this comment.
Not sure where to put this though, this might very well be the first place we can actually hook into.
JelteF
commented
Jan 14, 2025
| @@ -9015,6 +9128,45 @@ get_rule_expr(Node *node, deparse_context *context, | |||
| appendStringInfoString(buf, " := "); | |||
| get_rule_expr(refassgnexpr, context, showimplicit); | |||
| } | |||
| else if (IsA(sbsref->refexpr, Var) && pgduckdb_var_is_duckdb_row((Var*) sbsref->refexpr)) { | |||
There was a problem hiding this comment.
Needs a comment about why this is needed.
JelteF
commented
Jan 14, 2025
| @@ -11082,6 +11234,10 @@ get_const_expr(Const *constval, deparse_context *context, int showtype) | |||
| char *extval; | |||
| bool needlabel = false; | |||
|
|
|||
| if (pgduckdb_is_unresolved_type(constval->consttype)) { | |||
JelteF
commented
Jan 14, 2025
| @@ -12121,8 +12277,11 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context) | |||
| /* Print the relation alias, if needed */ | |||
| get_rte_alias(rte, varno, false, context); | |||
|
|
|||
| if (pgduckdb_func_returns_duckdb_row(rtfunc1)) { | |||
| /* Don't print column aliases for functions that return a duckdb.row */ | |||
There was a problem hiding this comment.
Why don't we do that?
JelteF
commented
Jan 14, 2025
src/pgduckdb_options.cpp
Outdated
| /* TODO: Fix this, if the column contains an array we should be able to subscript that type */ | ||
| elog(ERROR, "Subscripting duckdb.row is only supported with a single subscript"); | ||
| } | ||
| pprint(indirection); |
There was a problem hiding this comment.
Remove pprint here and in other places.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently when you use
read_parquet/read_csv/etc they require specifying the full type signature of the function call. This is quite cumbersome, especially when usingSELECT * FROM read_parquet(...) AS (...). UsingSELECT *its main advantage is that you don't have to specify all the columns.This changes that by instead allowing the following new syntax:
This syntax is made possible by using two clever hacks to solve two different problems:
The first problem is that Postgres its parser needs to consider the query valid, which means it has to resolve all the types. Sadly, we cannot hook into the type resolution as an extension. Instead the types need to be resolved fully from information in the Postgres catalogs. To do so we create a new type for our extension:
duckdb.unresolved_type. This type only exists for the Postgres parser and should never be used explicitly. We add catalog entries that allow this type be casted explicitly to any type that's supported by pg_duckdb. Using explicit casts always is quite annoying though. So we also define various operators and functions for this type, so that things liker['age'] > 21are allowed.The second problem is that the Postgres parser replaces the
*inSELECT *with an expanded column list. So the query above will look as follows once we receive it (*replaced withr):To resolve this problem we let
read_parquetreturn aduckdb.row, which we then replace with a*again before in ourpgduckdb_get_querydeffunction. This might sound pretty simple but there's tricks that need to be done for subqueries that return both aduckdb.rowtype and some other columns.TODO:
src/pgduckdb_ruleutils.cppwhich is shared across versions.