Skip to content

Commit

Permalink
Fix Antlr rewrite for WITHIN GROUP() for STRING_AGG() (babelfish-for-…
Browse files Browse the repository at this point in the history
…postgresql#2983)

### 1. Issues
- Rewriting logic in antlr of WITHIN GROUP() for STRING_AGG() was incorrect as it involved shifting of order_by_clause. This will cause a problem when there is rewrite in order_by_clause itself as now the index of expressions inside order_by_clause will change and this will result in incorrect rewrite.

### 2. Changes made to fix the issues
- Corrected Parser rewiting logic to avoid shifting of order_by_clause and only omit the unwanted phrases in between.
```
# Input
1> CREATE TABLE string_agg_t (id int, a varchar(10), b varchar(10), g int, sbid int)
2> INSERT INTO string_agg_t values (3,'c','x',1,4), (2,'b','y',2,6)
3> go
1> SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY trim(a) ASC) FROM string_agg_t GROUP BY g ORDER BY g
2> go
```
```
# previous output (wrong output)               
Msg 33557097, Level 16, State 1, Server BABELFISH, Line 1
syntax error at or near ")" 
```
```
# current output (correct output)

string_agg                                                                                                                                                                                                                                                      
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
cx                                                                                                                                                                                                                                                              
by                                                                                                                                                                                                                                     
```
- Added Test Cases for these issues

Task: BABEL-5290
Signed-off-by: Anikait Agrawal [[email protected]](mailto:[email protected])
  • Loading branch information
Anikait143 authored Sep 27, 2024
1 parent ba6df87 commit 0a7bedb
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
7 changes: 2 additions & 5 deletions contrib/babelfishpg_tsql/src/tsqlIface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8331,14 +8331,11 @@ rewrite_string_agg_query(TSqlParser::STRING_AGGContext *ctx)
{
if (ctx->WITHIN() && ctx->order_by_clause())
{
rewritten_query_fragment.emplace(std::make_pair(ctx->RR_BRACKET()[0]->getSymbol()->getStartIndex(), std::make_pair(::getFullText(ctx->RR_BRACKET()[0]), std::string(" ") + ::getFullText(ctx->order_by_clause()) + ::getFullText(ctx->RR_BRACKET()[0]))));

/* remove block (WITHIN GROUP LR_BRACKET order_by_clause RR_BRACKET) */
/* remove block (RR_BRACKET WITHIN GROUP LR_BRACKET) */
rewritten_query_fragment.emplace(std::make_pair(ctx->RR_BRACKET()[0]->getSymbol()->getStartIndex(), std::make_pair(::getFullText(ctx->RR_BRACKET()[0]), "")));
rewritten_query_fragment.emplace(std::make_pair(ctx->WITHIN()->getSymbol()->getStartIndex(), std::make_pair(::getFullText(ctx->WITHIN()), "")));
rewritten_query_fragment.emplace(std::make_pair(ctx->GROUP()->getSymbol()->getStartIndex(), std::make_pair(::getFullText(ctx->GROUP()), "")));
rewritten_query_fragment.emplace(std::make_pair(ctx->LR_BRACKET()[1]->getSymbol()->getStartIndex(), std::make_pair(::getFullText(ctx->LR_BRACKET()[1]), "")));
rewritten_query_fragment.emplace(std::make_pair(ctx->order_by_clause()->start->getStartIndex(), std::make_pair(::getFullText(ctx->order_by_clause()), "")));
rewritten_query_fragment.emplace(std::make_pair(ctx->RR_BRACKET()[1]->getSymbol()->getStartIndex(), std::make_pair(::getFullText(ctx->RR_BRACKET()[1]), "")));
}

if (ctx->STRING_AGG())
Expand Down
36 changes: 36 additions & 0 deletions test/JDBC/expected/string_agg_within-vu-verify.out
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,42 @@ s-gu-ev-by
~~END~~


SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY trim(a) ASC) FROM string_agg_t GROUP BY g ORDER BY g
GO
~~START~~
varchar
az-cx-dw-ht
s-by-ev-gu
~~END~~


SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY trim(a) DESC) FROM string_agg_t GROUP BY g ORDER BY g
GO
~~START~~
varchar
ht-dw-cx-az
gu-ev-by-s
~~END~~


SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY translate(a, 'a', 'b') ASC) FROM string_agg_t GROUP BY g ORDER BY g
GO
~~START~~
varchar
az-cx-dw-ht
s-by-ev-gu
~~END~~


SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY translate(a, 'a', 'b') DESC) FROM string_agg_t GROUP BY g ORDER BY g
GO
~~START~~
varchar
ht-dw-cx-az
gu-ev-by-s
~~END~~


SELECT STRING_AGG(a, char(10)) WITHIN GROUP (ORDER BY a ASC) FROM string_agg_t GROUP BY g ORDER BY g
GO
~~START~~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,18 @@ GO
SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY concat(a,b) DESC) FROM string_agg_t GROUP BY g ORDER BY g
GO

SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY trim(a) ASC) FROM string_agg_t GROUP BY g ORDER BY g
GO

SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY trim(a) DESC) FROM string_agg_t GROUP BY g ORDER BY g
GO

SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY translate(a, 'a', 'b') ASC) FROM string_agg_t GROUP BY g ORDER BY g
GO

SELECT STRING_AGG(concat(a,b),'-') WITHIN GROUP (ORDER BY translate(a, 'a', 'b') DESC) FROM string_agg_t GROUP BY g ORDER BY g
GO

SELECT STRING_AGG(a, char(10)) WITHIN GROUP (ORDER BY a ASC) FROM string_agg_t GROUP BY g ORDER BY g
GO

Expand Down

0 comments on commit 0a7bedb

Please sign in to comment.