Skip to content

Conversation

@sgrebnov
Copy link

@sgrebnov sgrebnov commented Oct 4, 2024

Which issue does this PR close?

PR improves unparsing to handle cases when ORDER BY clause contains aggregation functions that needs to be unprojected, otherwise it will result in unparsed/incorrect sql - Referenced column "sum(catalog_returns.cr_net_loss)" not found in FROM clause / Referenced column "count(DISTINCT cs1.cs_order_number)" not found in FROM clause

For example TPC-DS Q92

Referenced column "sum(catalog_returns.cr_net_loss)" not found in FROM clause

select  dt.d_year
 	,item.i_category_id
 	,item.i_category
 	,sum(ss_ext_sales_price)
 from 	date_dim dt
 	,store_sales
 	,item
 where dt.d_date_sk = store_sales.ss_sold_date_sk
 	and store_sales.ss_item_sk = item.i_item_sk
 	and item.i_manager_id = 1
 	and dt.d_moy=11
 	and dt.d_year=1998
 group by 	dt.d_year
 		,item.i_category_id
 		,item.i_category
 order by       sum(ss_ext_sales_price) desc,dt.d_year
 		,item.i_category_id
 		,item.i_category
 LIMIT 100 ;

This fixes unparsing for TPC-DS benchmark queries 16, 42, 62, 91, 92, 94, 95, 99

@sgrebnov sgrebnov self-assigned this Oct 4, 2024
@sgrebnov sgrebnov changed the title Improve ORDER BY with Aggregation functions unparsing Improve unparsing for ORDER BY with Aggregation functions Oct 4, 2024
Co-authored-by: Phillip LeBlanc <phillip@leblanc.tech>
@sgrebnov sgrebnov merged commit ee13eb0 into spiceai-42 Oct 4, 2024
@sgrebnov sgrebnov deleted the sgrebnov/improve-order_by branch October 4, 2024 21:05
Sevenannn pushed a commit that referenced this pull request Oct 26, 2024
…regation (apache#12946)

* Improve unparsing for ORDER BY with Aggregation functions (#38)

* Improve UNION unparsing (#39)

* Scalar functions in ORDER BY unparsing support (#41)

* Improve unparsing for complex Window functions with Aggregation (#42)

* WindowFunction order_by should respect `supports_nulls_first_in_sort` dialect setting (#43)

* Fix plan_to_sql

* Improve
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants