querybuilder
querybuilder copied to clipboard
Implement column-as keyword for 'select' with SelectAs().
This adds support for the following two syntaxes:
SelectAs()
Query().SelectAs(("Row1", "Alias1"), ("Row2", "Alias2")).From("Table");
--> "SELECT [Row1] AS [Alias1], [Row2] AS [Alias2] FROM [Table]"
Aggregates aliases
SelectAggregate()
supersedes AsAggregate()
, e.g. AsCount()
becomes SelectCount()
.
Query().SelectCount("Column", "Alias").From("Table");
--> "SELECT COUNT([Column]) AS [Alias] FROM [Table]"
Notes
-
While new function is designed as a drop-in replacement for the old one, we still opted to rename the existing aggregates from e.g.
AsCount()
toSelectCount()
, as we felt that this name more clearly communicates the intended use; to 'select' an 'aggregate' column. We considered keeping the old names around for compatibility, but then there would be two confusingly named functions with different capabilities and behaviour. -
We noticed that there is existing support for the
as
keyword, where a lower-caseas
in a column name is treated as theAS
keyword, which this PR leaves intact. However note that the existing way of handling 'as' is fragile. For example this does not work properly for aggregates, as in the following example.
Query().AsCount(new[] { "Column as Alias" }).From("Table");
--> "SELECT COUNT([Column] AS [Alias]) AS [count] FROM [Table]"
This yields invalid SQL syntax where there is an additional as
added within the count()
function.
When I see SelectCount
or similar functions, I get the intention that this is a shortcut for SelectRaw("count(...)")
method while this is not the case, the AsCount
here is the non executable version of XQuery
's Count()
function which execute the query and return the result immediately.
So all the AsX
methods here are just to inform the compiler that this query will be used as Count()
when executed so non necessary information can be trimmed out from the generated SQL.
I think keeping the name as is is more appropriate.
When I see
SelectCount
or similar functions, I get the intention that this is a shortcut forSelectRaw("count(...)")
method while this is not the case, theAsCount
here is the non executable version ofXQuery
'sCount()
function which execute the query and return the result immediately.So all the
AsX
methods here are just to inform the compiler that this query will be used asCount()
when executed so non necessary information can be trimmed out from the generated SQL.I think keeping the name as is is more appropriate.
This is a good point, I've rebased on master
and dropped the rename for now.
This rename makes more sense in the context of a future PR, which adds support for selecting aggregates as columns, so you can for example do:
select "Supplier", count("Customer") as "Num Customers"
from "Cases"
group by "Supplier"
order by "Num Customers" desc
We chose the SelectCount()
name more for this case, with the AsCount()
behaviour as a special case for only the case where the aggregation is COUNT
and more than two columns are selected.
For that PR, what would be a good way (name) to make the distinction between what AsCount()
does vs. selecting aggregates of columns?
--edit For context I've opened #504 which implements selecting multiple aggregate columns and should fix e.g. #203
@dgeelen-uipath I still see that the aggregate methods names are flipped like CountAs
instead of AsCount
or maybe I am browsing the wrong branch?
@dgeelen-uipath I still see that the aggregate methods names are flipped like
CountAs
instead ofAsCount
or maybe I am browsing the wrong branch?
No, you're right. This PR was intended as preparatory work for the follow up (#504), but it's diverging a bit it would seem (that's okay, but something to keep in mind). There were multiple renames in this PR; one to rename AsCount()
-> CountAs()
and then another to rename CountAs()
-> SelectCount()
. Only the latter rename was moved to #504 and I forgot to undo the CountAs()
rename here. Sorry for the confusion.
I've changed this PR as follows; I've kept the original functions (AsCount()
, AsSum()
, ...), but added new functions which support a column alias (AsCountAs()
, AsSumAs()
, ...). This should hopefully make it more clear what is going on with the code changes.
However, I'm personally not entirely satisfied with the result, I'm wondering if the alias support could be folded in to the existing functions as a default argument instead. The problem with that is that AsCount()
already takes a list of strings, which it uses to implement the counting of multiple columns, and thus there is no place to add the alias. I do believe that this is a problem only for COUNT
, since the other aggregates don't have the special case support for multiple columns, so perhaps we could make it an exception for AsCountAs()
only? It doesn't seem very uniform though, and maybe even doesn't help with the difference in behaviour between AsAggregate()
functions and the proposed SelectAggregate()
from #504 (which implements support for multiple aggregates per query).
Perhaps we can keep the naming for the AsCount()
/AsCountAs()
like here, and then introduce a name SelectCount()
(with alias support included) for the different behaviour implemented by #504. Another possibility could be to drop the alias support for the AsAggregate()
functions all together, and focus on supporting aliases for multiple-aggregates-per-query only.
What do you think?
P.S. I will update #504 once it's clear how best to deal with the naming, so for now it will remain a bit out-of-date.