Skip to content

Conversation

@sgrebnov
Copy link

@sgrebnov sgrebnov commented Oct 11, 2024

Which issue does this PR close?

Improves unparsing to correctly handle scalar functions in ORDER BY. If SELECT and ORDER BY contain the same expression with a scalar function, the ORDER BY expression will be replaced by a Column expression (e.g., "substr(customer.c_last_name, Int64(0), Int64(5))"), and we need to transform it back to the actual expression. This is done for scalar function expressions only.

  select c_last_name,c_first_name, substr(c_last_name,0,5)
  from customer
  order by substr(c_last_name,0,5)
  limit 10;

Before

select
	"customer"."c_last_name",
	"customer"."c_first_name",
	substr("customer"."c_last_name",0,5)
from
	"customer"
order by
	"substr(customer.c_last_name,Int64(0),Int64(5))" asc nulls last
limit 10

After

select
	"customer"."c_last_name",
	"customer"."c_first_name",
	substr("customer"."c_last_name",0,5)
from
	"customer"
order by
	substr("customer"."c_last_name",0,5) asc nulls last
limit 10

Foxes TPC-DS Benchmark Q85, Q79

Closes spiceai/spiceai#2962

ORDER BY `substr(reason.r_reason_desc,Int64(1),Int64(20))`

@sgrebnov sgrebnov changed the title Scalar functions in ORDER BY unparsing support Improve scalar functions in ORDER BY unparsing Oct 11, 2024
@phillipleblanc phillipleblanc merged commit 5f38df6 into spiceai-42 Oct 12, 2024
@phillipleblanc phillipleblanc deleted the sgrebnov/improve-orderby-unparsing branch October 12, 2024 03:30
@sgrebnov sgrebnov self-assigned this Oct 25, 2024
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.

3 participants