Skip to content
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

Preserve the name of column after unnest #11193

Closed
wants to merge 1 commit into from

Conversation

oneonestar
Copy link
Contributor

Motivation

Before #10883, we were able to do the following selection:
SELECT a.field1, a.field2 FROM UNNEST(...) t(a)

After the patch, this kind of query become impossible. The name of the fields becomes _col0, _col1. Column aliases is required in order to select the needed columns. We could rewrite it in this way:
SELECT field1, field2 FROM UNNEST(...) t(field1, field2)

However, it is infeasible to use column aliases to refer to a row with a lot of fields. I think we should preserve the name of the columns so that we can refer to the column easily, as below:

# Hard to use column aliases for complicated row (after #10883):
SELECT field1, field100 FROM UNNEST(...) t(field1, field2, field3, ..., field100)

# Preserve the name of the columns will allow this:
SELECT field1, field100 FROM UNNEST(...) t

What changes were proposed in this pull request

Before:

presto> SELECT * FROM UNNEST(array[
     ->     CAST(ROW('a', 1) AS ROW(x varchar, y int)),
     ->     CAST(ROW('a', 1) AS ROW(x varchar, y int))
     -> ]);
 _col0 | _col1
-------+-------
 a     |     1
 a     |     1
(2 rows)

After:

presto> SELECT * FROM UNNEST(array[
     ->     CAST(ROW('a', 1) AS ROW(x varchar, y int)),
     ->     CAST(ROW('a', 1) AS ROW(x varchar, y int))
     -> ]);
 x | y
---+---
 a | 1
 a | 1
(2 rows)

@martint
Copy link
Contributor

martint commented Aug 6, 2018

Indeed, this is a bug in #10883. According to the SQL specification:

ii) Otherwise, Case:
  1) If any ETj, 1 (one) ≤ j ≤ NCV, is a row type, then:
    A) For each ETj that is a row type:
      I) Let DET(j) be the degree of ETj.
>>>>  II) Let FNj,i, 1 (one) ≤ i ≤ DET(j), be the name of the i-th field in ETj. 

Copy link
Contributor

@martint martint left a comment

Choose a reason for hiding this comment

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

Overall, looks good, but please adjust the test cases based on the comments.

public void testUnnestPreserveColumnName()
{
assertions.assertQuery(
"SELECT x FROM UNNEST(array[" +
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not immediately obvious that array(cast(...) as row(x int, y varchar), cast(...) as row (x int, y varchar))) is of type
array(row(x int, y varchar)).

A better form would be: cast(array[row(1, 'a'), row(2, 'b')] as array(row(x int, y varchar)))

@martint
Copy link
Contributor

martint commented Aug 6, 2018

Ok, I made the changes I suggested above and merged it. Thanks for you contribution!

@martint martint closed this Aug 6, 2018
@oneonestar oneonestar deleted the unnest_name branch August 7, 2018 00:32
@oneonestar oneonestar restored the unnest_name branch August 7, 2018 04:48
@oneonestar oneonestar deleted the unnest_name branch August 7, 2018 04:49
@oneonestar oneonestar restored the unnest_name branch August 7, 2018 04:51
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.

None yet

3 participants