-
Notifications
You must be signed in to change notification settings - Fork 26
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
possible to support a two-argument pggen.arg eg pggen.arg(param text, default_value anyelement)
?
#86
Comments
ppgen.arg(param text, default_value anyelement)
ppgen.arg(param text, default_value anyelement)
?
The alternative syntax for params suggested in #42 is slowly growing on me. As for what I personally use: some editors allow you to define a regexp for a customizable parameter syntax. Here's how to do it for IntelliJ-based editors: Run pggen queries directlyInstead of copying queries, you can run pggen queries directly against any IntelliJ datasource by defining a regexp to match First, create a new parameter pattern with the regexp.
Does that approach work for you? |
I appreciate the workflow info! The jetbrains rewrite rule seems like an 'ok' workaround but that approach preserves a number of pretty annoying papercuts ...
As I consider your suggestion and think about the two arg approach again, I'm even more leaning towards the two arg approach ... Consider a workflow like this:
This approach could maybe also provide a solution for -- #85 ... I think it could make sense for the type inference for the two argument eg -- If this approach was embraced, maybe there could even future features added building on this -- maybe a special comment directive for the declaration of |
Funnily enough, I initially prototyped support for pggen.arg with a default value but never exposed it: 363cf19 The reasons I haven't implemented pggen.arg with a default:
SELECT
nullif(pggen.arg('foo'), ''), -- to use a null default value
coalesce(nullif(pggen.arg('foo'), ''), 'default_value) -- to use a default value
I think using the At the design level, I'm wary of expanding Pggen's syntax to support interactive use-case. I'm sympathetic since I've done regexp surgery to convert pggen queries to support interactive queries. AsideI don't think the following helps the interactive use-case but it gives an idea for pggen's future direction: Long term, I think the way forward is more user-configurable type-mappers at three levels. At present, type overrides are only supported at the invocation level.
Then a user can use different type mappers to support null or default values. I should start a pggen "v2" tracking ticket for these ideas. |
Don't you ever run into cases where you have an argument with a zero-value which is inappropriate for the query ...? The
Maybe I'm wrong here ... but could a simple approach work here? Keep count of the non-string-non-comment-enclosed open/close parenthesis after the ',' until finding a closing paren which represents the end of the |
I made a sketch of what I was thinking -- breathe@fece1fe That implementation puts logic into the tokenizer to scan Maybe an approach along these lines could work without a huge amount of additional complexity ...? (Note the existing scanner and parser tests pass but I didn't exercise the two-argument version at all ... am just wondering what you think of this approach / whether you would consider accepting a change like this ...) |
ppgen.arg(param text, default_value anyelement)
?pggen.arg(param text, default_value anyelement)
?
Thanks for experimenting with the code! Adding a It seems like the main problem is there's no way to pass If pggen could pass
Let me know if that matches your understanding. If it does, I'd prefer to figure out how to pass null values via |
But the case statement example only supports string typed arguments. afaict the two arg version is needed for the interactive use case as it's the only way to be able to define a pggen.arg() stub that can return values for arguments with arbitrary types associated with the arg name (?) I still see value in the two arg version of the directive ... even with the addition of more type customization for argument types -- it would still be nice to have a version of the directive that always infers an optional type ... the two arg version could behave that way ... pggen.arg('foo') -- keeps current implementation pggen.arg('foo', null::int) -- infers optional type for 'foo', compiles to 'coalesce(:1, null::int)' This could be added without breaking backward compatibility I think ... And I don't think this enhancement would add any major surprises to the implementation of any of the proposed new type inference customization features ...? (even with the ability to customize the inferred go argument types, I'd prefer not writing those in general -- especially if the only reason I have to write them is to get an optional type ...) |
I went ahead and wrote a PR to implement ... Let me know what you think if you have a chance? Thanks! |
Closing as I gather there is not interest in supporting the interactive sql use case |
I came across pggen today and I'm enjoying it so far - I like static queries and the vision here.
One pain point though is that I would like to be able to directly execute my sql queries for ease of testing and prototyping -- and I need some way of supplying appropriate values and value_types to
ppgen.arg()
.I initially did something like this
The idea there was to define a fake
pggen.arg()
method which returns different string values based on the providedparam
. This gives a workflow for interactively prototyping a query -- and when want to test different query arguments I just change the value in the fake definition ofpggen.arg()
and re-execute the query.There are two problems with that:
I'm wondering if we could add support to pggen for a two argument version of
pggen.arg
-- like below:This would allow for writing queries that are consumable by pggen but which cann still be executed interactively for ease of development and feedback speed.
For the above, when the query is run interactively
pggen.arg(text, integer[])
will be invoked and the integer[] value will be returned, meanwhile the compiled query will take the value of the argument from the corresponding prepared statement placeholder as normal.The text was updated successfully, but these errors were encountered: