Skip to content

Fix invalid plan for repeated lambdas in order by#13901

Merged
rschlussel merged 1 commit intoprestodb:masterfrom
rschlussel:orderby-lambda
Jan 9, 2020
Merged

Fix invalid plan for repeated lambdas in order by#13901
rschlussel merged 1 commit intoprestodb:masterfrom
rschlussel:orderby-lambda

Conversation

@rschlussel
Copy link
Contributor

@rschlussel rschlussel commented Dec 30, 2019

Presto was creating invalid plans for queries with duplicate lambda
expressions in the order by clause because the expressions were
considered "equal" for some purposes, but not equal when converting the lambdas
to symbols. This caused one of the expressions to not be mapped to an input
symbol. To resolve this, we don't deduplicate even seemingly equal expressions
in the order by clause so that we maintain translations for all the
expressions.

Example query:

SELECT
  COUNT(*)
FROM
  (values ARRAY['a', 'b']) as t(col1)
ORDER BY
  IF(
    SUM(
      REDUCE(
        col1,
        ROW(0),
        ("l", "r") -> "l",
        "x" -> 1
      )
    ) > 0,
    COUNT(*),
    SUM(
      REDUCE(
        col1,
        ROW(0),
        ("l", "r") -> "l",
        "x" -> 1
      )
    )
  )

This fixes the second part of #10694

== RELEASE NOTES ==

General Changes
* Fix failures caused by invalid plans for queries with repeated lambda expressions in the order by clause.

@rschlussel rschlussel requested a review from rongrong December 30, 2019 15:38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just do

        stream(expressionsToRemap)
                .forEach(expression -> newBuilder.getTranslations().put(expression, builder.translate(expression)));

I don't have strong opinions. Just that overall the codebase seems to prefer stream API when logic is simple.

Presto was creating invalid plans for queries with duplicate lambda
expressions in the order by clause because the expressions were
considered "equal" for some purposes, but not equal when converting the lambdas
to symbols.  This caused one of the expressions to not be mapped to an input
symbol. To resolve this, we don't deduplicate even seemingly equal expressions
in the order by clause so that we maintain translations for all the
expressions.

Example query:
SELECT
  COUNT(*)
FROM
  (values ARRAY['a', 'b']) as t(col1)
ORDER BY
  IF(
    SUM(
      REDUCE(
        col1,
        ROW(0),
        ("l", "r") -> "l",
        "x" -> 1
      )
    ) > 0,
    COUNT(*),
    SUM(
      REDUCE(
        col1,
        ROW(0),
        ("l", "r") -> "l",
        "x" -> 1
      )
    )
  )
@rschlussel rschlussel merged commit fd59547 into prestodb:master Jan 9, 2020
@aweisberg aweisberg mentioned this pull request Jan 17, 2020
7 tasks
@caithagoras caithagoras mentioned this pull request Jan 22, 2020
6 tasks
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