-
-
Notifications
You must be signed in to change notification settings - Fork 262
New function LISTAGG #8689
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
base: master
Are you sure you want to change the base?
New function LISTAGG #8689
Changes from 3 commits
640b352
9f34905
72a977f
dbf81d6
dfcbe9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,144 @@ | ||||||
| SQL Language Extension: LISTAGG | ||||||
|
|
||||||
| Function: | ||||||
| The current implementation has an aggregate function LIST which concatenates multiple row | ||||||
| fields into a blob. The SQL standard has a similar function called LISTAGG. The major | ||||||
| difference is that it also supports the ordered concatenation. | ||||||
|
|
||||||
| Authors: | ||||||
| Chudaykin Alex <[email protected]> | ||||||
|
|
||||||
| Format: | ||||||
| <listagg set function> ::= | ||||||
| LISTAGG <left paren> [ <set quantifier> ] <character value expression> <comma> <listagg separator> [ <listagg overflow clause> ] <right paren> <within group specification> | ||||||
|
|
||||||
| <listagg separator> ::= | ||||||
| <character string literal> | ||||||
|
|
||||||
| <listagg overflow clause> ::= | ||||||
| ON OVERFLOW <overflow behavior> | ||||||
|
|
||||||
| <overflow behavior> ::= | ||||||
| ERROR | TRUNCATE [ <listagg truncation filler> ] <listagg count indication> | ||||||
|
|
||||||
| <listagg truncation filler> ::= | ||||||
| <character string literal> | ||||||
|
|
||||||
| <listagg count indication> ::= | ||||||
| WITH COUNT | WITHOUT COUNT | ||||||
|
|
||||||
| <within group specification> ::= | ||||||
| WITHIN GROUP <left paren> ORDER BY <sort specification list> <right paren> | ||||||
|
|
||||||
| Syntax Rules: | ||||||
| The legacy LIST syntax is preserved for backward compatibility, LISTAGG is added to cover the | ||||||
| standard features. | ||||||
|
|
||||||
| There is a <listagg overflow clause> rule in the standard, which is intended to output an error | ||||||
| when the output value overflows. Since the LIST function always returns a BLOB, it was decided | ||||||
| that this rule would be meaningless. It was not implemented and silently ignored if specified. | ||||||
|
|
||||||
| If DISTINCT is specified for LISTAGG, then ORDER BY <sort specification list> must fully match | ||||||
| <character value expression> | ||||||
|
|
||||||
| Notes: | ||||||
| If DISTINCT is specified, the presence of WITHIN GROUP must obey the restriction and will not | ||||||
| affect the subsequent code execution. | ||||||
|
|
||||||
| Examples: | ||||||
| CREATE TABLE TEST_T | ||||||
| (COL1 INT, COL2 VARCHAR(2), COL3 VARCHAR(2), COL4 VARCHAR(2), COL5 BOOLEAN, COL6 VARCHAR(2) | ||||||
| CHARACTER SET WIN1251); | ||||||
| COMMIT; | ||||||
| INSERT INTO TEST_T values(1, 'A', 'A', 'J', false, 'П'); | ||||||
| INSERT INTO TEST_T values(2, 'B', 'B', 'I', false, 'Д'); | ||||||
| INSERT INTO TEST_T values(3, 'C', 'A', 'L', true, 'Ж'); | ||||||
| INSERT INTO TEST_T values(4, 'D', 'B', 'K', true, 'Й'); | ||||||
| COMMIT; | ||||||
|
|
||||||
| SELECT LISTAGG (ALL COL4, ':') AS FROM TEST_T; | ||||||
| ======= | ||||||
| J:I:L:K | ||||||
|
|
||||||
| SELECT LISTAGG (DISTINCT COL4, ':') FROM TEST_T; | ||||||
| ======== | ||||||
| I:J:K:L | ||||||
|
|
||||||
| SELECT LISTAGG (DISTINCT COL3, ':') FROM TEST_T; | ||||||
| ==== | ||||||
| A:B | ||||||
|
|
||||||
| SELECT LISTAGG (DISTINCT COL3, ':') WITHIN GROUP (ORDER BY COL2) FROM TEST_T; | ||||||
| ==== | ||||||
| A:B | ||||||
|
|
||||||
| SELECT LISTAGG (DISTINCT COL3, ':') WITHIN GROUP (ORDER BY COL2 DESCENDING) FROM TEST_T; | ||||||
| ==== | ||||||
| A:B | ||||||
|
|
||||||
| SELECT LISTAGG (COL2, ':') WITHIN GROUP (ORDER BY COL2 DESCENDING) FROM TEST_T; | ||||||
| ======= | ||||||
| D:C:B:A | ||||||
|
|
||||||
| SELECT LISTAGG (COL4, ':') WITHIN GROUP (ORDER BY COL3 DESC) FROM TEST_T; | ||||||
| ======= | ||||||
| I:K:J:L | ||||||
|
|
||||||
| SELECT LISTAGG (COL3, ':') WITHIN GROUP (ORDER BY COL5 ASCENDING) FROM TEST_T; | ||||||
| ======= | ||||||
| A:B:A:B | ||||||
|
|
||||||
| SELECT LISTAGG (COL4, ':') WITHIN GROUP (ORDER BY COL3 ASC) FROM TEST_T; | ||||||
| ======= | ||||||
| J:L:I:K | ||||||
|
|
||||||
| SELECT LISTAGG (ALL COL2) WITHIN GROUP (ORDER BY COL4) FROM TEST_T; | ||||||
| ======= | ||||||
| B,A,D,C | ||||||
|
|
||||||
| SELECT LISTAGG (COL2, ':') WITHIN GROUP (ORDER BY COL3 DESC, COL4 ASC) FROM TEST_T; | ||||||
| ======= | ||||||
| B:D:A:C | ||||||
|
|
||||||
| SELECT LISTAGG (COL2, ':') WITHIN GROUP (ORDER BY COL3 DESC, COL4 DESC) FROM TEST_T; | ||||||
| ======= | ||||||
| D:B:C:A | ||||||
|
|
||||||
| SELECT LISTAGG (COL2, ':') WITHIN GROUP (ORDER BY COL3 ASC, COL4 DESC) FROM TEST_T; | ||||||
| ======= | ||||||
| C:A:D:B | ||||||
|
|
||||||
| SELECT LISTAGG (ALL COL6, ':')FROM TEST_T; | ||||||
|
||||||
| SELECT LISTAGG (ALL COL6, ':')FROM TEST_T; | |
| SELECT LISTAGG (ALL COL6, ':') FROM TEST_T; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -124,6 +124,7 @@ string AggNode::internalPrint(NodePrinter& printer) const | |||||||||||||||||
| NODE_PRINT(printer, dialect1); | ||||||||||||||||||
| NODE_PRINT(printer, arg); | ||||||||||||||||||
| NODE_PRINT(printer, asb); | ||||||||||||||||||
| NODE_PRINT(printer, sort); | ||||||||||||||||||
| NODE_PRINT(printer, indexed); | ||||||||||||||||||
|
|
||||||||||||||||||
| return aggInfo.name; | ||||||||||||||||||
|
|
@@ -307,7 +308,7 @@ bool AggNode::dsqlMatch(DsqlCompilerScratch* dsqlScratch, const ExprNode* other, | |||||||||||||||||
| // ASF: We compare name address. That should be ok, as we have only one AggInfo instance | ||||||||||||||||||
| // per function. | ||||||||||||||||||
| return aggInfo.blr == o->aggInfo.blr && aggInfo.name == o->aggInfo.name && | ||||||||||||||||||
| distinct == o->distinct && dialect1 == o->dialect1; | ||||||||||||||||||
| distinct == o->distinct && dialect1 == o->dialect1 && sort == o->sort;; | ||||||||||||||||||
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| void AggNode::setParameterName(dsql_par* parameter) const | ||||||||||||||||||
|
|
@@ -352,6 +353,8 @@ AggNode* AggNode::pass2(thread_db* tdbb, CompilerScratch* csb) | |||||||||||||||||
| dsc desc; | ||||||||||||||||||
| getDesc(tdbb, csb, &desc); | ||||||||||||||||||
| impureOffset = csb->allocImpure<impure_value_ex>(); | ||||||||||||||||||
| if (sort) | ||||||||||||||||||
| doPass2(tdbb, csb, sort.getAddress()); | ||||||||||||||||||
|
|
||||||||||||||||||
| return this; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
@@ -361,7 +364,7 @@ void AggNode::aggInit(thread_db* tdbb, Request* request) const | |||||||||||||||||
| impure_value_ex* impure = request->getImpure<impure_value_ex>(impureOffset); | ||||||||||||||||||
| impure->vlux_count = 0; | ||||||||||||||||||
|
|
||||||||||||||||||
| if (distinct) | ||||||||||||||||||
| if (distinct || sort) | ||||||||||||||||||
| { | ||||||||||||||||||
| // Initialize a sort to reject duplicate values. | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -373,8 +376,8 @@ void AggNode::aggInit(thread_db* tdbb, Request* request) const | |||||||||||||||||
|
|
||||||||||||||||||
| asbImpure->iasb_sort = FB_NEW_POOL(request->req_sorts.getPool()) Sort( | ||||||||||||||||||
| tdbb->getDatabase(), &request->req_sorts, asb->length, | ||||||||||||||||||
| asb->keyItems.getCount(), 1, asb->keyItems.begin(), | ||||||||||||||||||
| RecordSource::rejectDuplicate, 0); | ||||||||||||||||||
| asb->keyItems.getCount(), (distinct ? 1 : asb->keyItems.getCount()), | ||||||||||||||||||
| asb->keyItems.begin(), (distinct ? RecordSource::rejectDuplicate : nullptr), 0); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -427,6 +430,44 @@ bool AggNode::aggPass(thread_db* tdbb, Request* request) const | |||||||||||||||||
| ULONG* const pDummy = reinterpret_cast<ULONG*>(data + asb->length - sizeof(ULONG)); | ||||||||||||||||||
| *pDummy = asbImpure->iasb_dummy++; | ||||||||||||||||||
|
|
||||||||||||||||||
| return true; | ||||||||||||||||||
| } | ||||||||||||||||||
| else if (sort) | ||||||||||||||||||
| { | ||||||||||||||||||
| fb_assert(asb); | ||||||||||||||||||
| // "Put" the value to sort. | ||||||||||||||||||
| impure_agg_sort* asbImpure = request->getImpure<impure_agg_sort>(asb->impure); | ||||||||||||||||||
| UCHAR* data; | ||||||||||||||||||
| asbImpure->iasb_sort->put(tdbb, reinterpret_cast<ULONG**>(&data)); | ||||||||||||||||||
|
|
||||||||||||||||||
| MOVE_CLEAR(data, asb->length); | ||||||||||||||||||
|
|
||||||||||||||||||
| auto descOrder = asb->descOrder.begin(); | ||||||||||||||||||
| auto keyItem = asb->keyItems.begin(); | ||||||||||||||||||
|
|
||||||||||||||||||
| for (auto& nodeOrder : sort->expressions) | ||||||||||||||||||
| { | ||||||||||||||||||
| dsc toDesc = *(descOrder++); | ||||||||||||||||||
| toDesc.dsc_address = data + (IPTR)toDesc.dsc_address; | ||||||||||||||||||
|
||||||||||||||||||
| toDesc.dsc_address = data + (IPTR)toDesc.dsc_address; | |
| toDesc.dsc_address = data + (IPTR) toDesc.dsc_address; |
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.
| if (IS_INTL_DATA(fromDsc)) | |
| INTL_string_to_key(tdbb, INTL_TEXT_TO_INDEX(fromDsc->getTextType()), | |
| fromDsc, &toDesc, INTL_KEY_UNIQUE); | |
| if (IS_INTL_DATA(fromDsc)) | |
| { | |
| INTL_string_to_key(tdbb, INTL_TEXT_TO_INDEX(fromDsc->getTextType()), | |
| fromDsc, &toDesc, INTL_KEY_UNIQUE); | |
| } |
Outdated
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.
| toDesc.dsc_address = data + (IPTR)toDesc.dsc_address; | |
| toDesc.dsc_address = data + (IPTR) toDesc.dsc_address; |
Outdated
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.
| desc.dsc_address = data + (IPTR)asb->desc.dsc_address; | |
| desc.dsc_address = data + (IPTR) asb->desc.dsc_address; |
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 will fail while parsing the BLR restored from the previous versions, because blr_sort will be missing.
One solution is to add blr_agg_list2 and use it only when the ordering is specified. Another would could be something like this:
if (csb->csb_blr_reader.peekByte() == blr_sort)
node->sort = PAR_sort(tdbb, csb, blr_sort, true);In this case, I'd also change genBlr() below to call GEN_sort() only if the ordering is specified -- this way we keep generating a compatible BLR for the legacy LIST syntax, making it possible to downgrade the database if required.
Outdated
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.
Should they be identical? Why?
I think it should have like GROUP BY rules.
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.
Good point. Currently, we sort only once and this is a good bonus. If we follow your suggestion and allow slightly different expressions, then we should either sort twice or ignore the user-specified ordering after DISTINCT. BTW, in this PR it also seems to be ignored -- LISTAGG(DISTINCT COL) WITHIN GROUP (ORDER BY COL DESC) would produce ASC-ordered output. But I suppose it should work the same way as for plain SELECT DISTINCT(COL) FROM T ORDER BY COL DESC, i.e. respect the ORDER BY ordering and optimize the sorts (merge two sorts into one) only if they fully (expressions / directions / NULLs placement) match each other.
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.
Why direction and null placement are important for DISTINCT?
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.
Nulls placement is not important, I agree, as NULLs are skipped by all aggregate functions.
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.
By standard, DISTINCT eliminates duplicates in the ordered (if specified) result set, it should not change the user-defined ordering. Why do you think LISTAGG should behave differently?
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.
It shouldn't but I don't quite understand why you said
i.e. respect the ORDER BY ordering and optimize the sorts (merge two sorts into one) only if they fully (expressions / directions / NULLs placement) match each other.
BTW, IMHO, sort->expressions.getCount() > 1 condition here is not needed as it is fine for distinct to use only first sorting segment.
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.
Ah, sorry, my bad. Surely, direction for the combined sort should be taken from ORDER BY -- like we already do in Optimizer::checkSorts().
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.
Nulls placement is not important, I agree, as NULLs are skipped by all aggregate functions.
With my suggestion to use same existing rules, the DISTINCT LISTAGG expression may be something like COALESCE(field, 'z') with a ORDER BY Z`.
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.
Given that we support queries like:
select distinct emp_no from employee order by last_name
I'm not sure we need to introduce the GROUP BY rules inside LISTAGG (DISTINCT ...) ... WITHIN GROUP. I suppose DISTINCT + ORDER BY should work the same way as either a standalone construct or within the LISTAGG group, i.e. without any restrictions. That said, I see no practical point in the sort behind the ORDER BY <other expression> clause because its result is not going to be user-visible anyway. It basically means that if DISTINCT is present, we may simply ignore the ORDER BY clause -- except when ORDER BY mentions the aggregated expression in the very beginning of its <sort specification list> -- then its sort direction should be taken into account inside DISTINCT.
Do I miss anything?
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.
In the syntax format section,
<within group specification>is mandatory but does not exist in some examples.Uh oh!
There was an error while loading. Please reload this page.
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.
The SQL specification declares
<within group specification>as mandatory. However, IMHO it's quite restrictive and neither Oracle nor DB2 follows that rule, they have it optional. Given thatLISTandLISTAGGshare the same syntax in this PR, we've also made<within group specification>optional. So the easiest solution is to fix the README ;-)Or we may go the standard way and separate the legacy
LIST(leave it with the current grammar, without ordering) fromLISTAGG(which is strictly standard-compliant) at the parser level. But IMHO it would be annoying for users to select either of them depending on whether you need ordering or not. So personally I'd keep everything "as is" and just fix the docs.Other opinions?
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 agree,
LISTandLISTAGGshould be complete synonyms.I would also remove the mention of
ON OVERFLOWfrom the documentation. It's standard, but we don't support it. It could be mentioned if it were simply ignored, but mentioning it leads to errors.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.
Strange. I need to check it out
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.
If we remove
ON OVERFLOWfrom the docs, then I believe we should remove it from the parser too.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.
ON OVERFLOWcan be kept if it will not cause errors.