-
Notifications
You must be signed in to change notification settings - Fork 217
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
issue when using a DuckDb Macro #4150
Comments
Right now I seem to be "forced" to use an s-string which is not very neat, eg
or something, is there a better way? |
similarly if I have a field with a "reserved name" like "timestamp" I cannot directly use it, I need to use s"timestamp", for instance
isn't there a way to see that what follows is necessarily a value, and should be coerced to a value? |
Hi @maelp , good questions, thanks for opening an issue. Check out https://prql-lang.org/book/reference/syntax/keywords.html — these are solvable with backticks and
SELECT
COALESCE(SUM(time), 0)
FROM
"cse(2024,1,5)"
-- Generated by PRQL compiler version:0.11.1 (https://prql-lang.org)
Does that make sense? |
Thanks! However if I use this DuckDB MACRO
I think it doesn't create a "table" per-se, but should be used as do you think I should instead create a table? not sure if I can do this as a macro / function? When I use your code I get the error:
|
The generated SQL is this
I guess it should not use the " character because I want the macro to be run in duckdb no? |
I verified that if I run the same query without the quotes, eg not eg
is there a way to generate this simply from PRQL? |
And if I try
s-strings representing a table must start with SELECT
|
Ah, so we want literally In that case, I think we do need to use the We do have prior art for something more with the This probably isn't going to make this perfect immediately, but it would be possible to make a function so the inline call were elegant: prql target:sql.duckdb
let cse = x y z -> s"select * from cse({x},{y},{z})"
from (cse 1 2 3)
derive x = 5 WITH table_0 AS (
SELECT
*
from
cse(1, 2, 3)
)
SELECT
*,
5 AS x
FROM
table_0
-- Generated by PRQL compiler version:0.11.1 (https://prql-lang.org) |
would be interesting if we could have something like this! Or allow macros and duckdb functions in PRQL or something |
BTW is it possible to do a multi-line PRQL function (this does not seem to be shown in the documentation) I'm trying to do a helper function, then call it like this
but it doesn't work... |
The let checkTemps = numWeeks minCellTemp maxCellTemp maxBMSTemp -> (
from (recent_cse)
derive {
min_temp_cells = s"LEAST(value.temperature_front, value.temperature_back)",
max_temp_cells = s"GREATEST(value.temperature_front, value.temperature_back)",
max_temp_bms = s"GREATEST(value.temperature_mosfet, value.temperature_ic)",
}
filter min_temp_cells < minCellTemp || max_temp_cells > maxCellTemp || max_temp_bms > maxBMSTemp
group bms_id (
aggregate {
min_temp_cells = min min_temp_cells,
max_temp_cells = max max_temp_cells,
max_temp_bms = max max_temp_bms,
count = count this,
}
)
)
checkTemps 4 (-10) 40 70 ...works! (also I changed Parentheses can be a bit confusing in some limited circumstances — explained here: https://prql-lang.org/book/reference/syntax/operators.html#parentheses. The main thing we can do is to make the error messages much much better. |
We could have something very very simple like How do you / others think about macros vs functions? To the extent they're overlapping, I would deprioritize work here — having a boilerplate function for each macro doesn't seem too bad. To the extent they'll be coexisting lots, we could think about improving the interface. |
Would be helpful to have a better help message indeed! I thought the issue was something else yes the boilerplate can work I agree, so not a big priority. It's just that I want to be able to do queries both in SQL and PRQL so I'd like to have macros available in both to avoid duplicating code would it be possible to rewrite that macro in pure PRQL (eg do you have access to duckdb generate_series and list_transform in PRQL? possibly in order to have those available, having something like a ::duckdb:generate_series and ::duckdb:list_transform as tokens (or a kind of PRQL "module" that we could import like "import duckdb" and then "duckdb.generate_series", "duckdb.list_transform" and "duckdb.call_macro" could be useful?) |
Maybe you want to see this. |
Sorry to use this issue to ask questions but I find that the documentation doesn't show all the power of PRQL yet, for instance: is there a way to do a ternary operation or an if / then / else in PRQL? I'd like to do something like
is that possible? |
You can use case https://prql-lang.org/book/reference/syntax/case.html |
Thanks! Last question (and small "bug report") when I use this it works fine
But if I'd like to "rephrase" the min_temp, max_temp, etc at the end to only display if they are above / under their respective threshold it fails, and the error message is weird (it complains about the "->" in the function definition)
(another small nitpick: why not using the same "->" for case and function, and introducing another symbol like "=>" ?) |
I think you forgot the comma in arrays. case [
foo # bad line
bar
] |
ah indeed you're right, but the error message is confusing, @max-sixty would there be a way to show the "correct" error in those cases? |
It's definitely possible. I'd say it's harder than I thought it would be when I originally proposed terser syntax... For example, in that case we need to realize we're in a list, possibly attempt to execute a function I would really like to focus on this, and think this will be the focus of our GSOC application. |
FYI there was some discussion on this in a past issue... I don't think anyone had a really strong view, we ended up favoring consistency with other langs |
What happened?
I have a macro defined in DuckDb to access Parquet files on s3
in SQL I can do
SELECT count(*) FROM cse(2024, 1, 5);
but if I try to compile the following PRQL
it gives an error
PRQL input
see above
SQL output
Expected SQL output
No response
MVCE confirmation
Anything else?
No response
The text was updated successfully, but these errors were encountered: