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

sql: column names not being passed correctly #36501

Closed
roncrdb opened this issue Apr 3, 2019 · 10 comments

Comments

6 participants
@roncrdb
Copy link

commented Apr 3, 2019

Describe the problem

In psql if you give an alias to a column it will pass that name along:

FROM (
    SELECT unnest(ARRAY[439889519552299009]) AS filter_id2
) AS uq
JOIN (
    SELECT unnest(ARRAY[439889519552299009]) AS filter_id
) AS ab
ON uq.filter_id2 = ab.filter_id;
     filter_id2     |     filter_id
--------------------+--------------------
 439889519552299009 | 439889519552299009

In CRDB, with a single column, we do not pass the name correctly:

root@:26257/test> SELECT DISTINCT *   FROM ( SELECT unnest(ARRAY[439889519552299009]) AS filter_id2, 1 AS hello) AS uq JOIN (SELECT unnest(ARRAY[439889519552299009]) AS filter_id, 2 AS goodbye) AS ab ON uq.filter_id2 = ab.filter_id;
      filter_id2     | hello |     filter_id      | goodbye
+--------------------+-------+--------------------+---------+
  439889519552299009 |     1 | 439889519552299009 |       2
(1 row)

Time: 1.993ms

root@:26257/test> SELECT DISTINCT *FROM (SELECT unnest(ARRAY[439889519552299009]) AS filter_id2, 1 AS hello) AS uq JOIN (SELECT unnest(ARRAY[439889519552299009]) AS filter_id) AS ab ON uq.filter_id2 = ab.filter_id;

pq: column "ab.filter_id" does not exist

root@:26257/test> SELECT DISTINCT * FROM ( SELECT unnest(ARRAY[439889519552299009]) AS filter_id2, 1 AS hello) AS uq JOIN (SELECT unnest(ARRAY[439889519552299009]) AS filter_id) AS abON uq.filter_id2 = ab;
      filter_id2     | hello |         ab
+--------------------+-------+--------------------+
  439889519552299009 |     1 | 439889519552299009
(1 row)

Time: 6.792ms

To Reproduce

What did you do? Describe in your own words.

If possible, provide steps to reproduce the behavior:

  1. Set up CockroachDB cluster ...
  2. Send SQL ... / CLI command ...
  3. Look at UI / log file / client app ...
  4. See error

Expected behavior
Column names should be passed correctly in order to avoid error message that column doesn't exist.

Environment:

  • CockroachDB v2.1.6
@tim-o

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2019

Zendesk ticket #3239 has been linked to this issue.

@jordanlewis

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

This appears to be a regression in the CBO. Here's a reproduction:

[17:50]% ./cockroach demo                                                                                                                                                                                                                                                 jordan@Termato:~/repo/src/github.com/cockroachdb/cockroach on dont-save-tmp -> mine/dont-save-tmp
#
# Welcome to the CockroachDB demo database!
#
# You are connected to a temporary, in-memory CockroachDB
# instance. Your changes will not be saved!
#
# Web UI: http://127.0.0.1:58580
#
# Server version: CockroachDB CCL v19.1.0-beta.20190318-428-g93f796457a-dirty (x86_64-apple-darwin18.2.0, built 2019/03/28 22:28:09, go1.11) (same version as client)
# Cluster ID: cb9ce44e-7752-4d3d-b630-b65b861de820
#
# Enter \? for a brief introduction.
#
root@127.0.0.1:58579/defaultdb> SELECT * FROM (                                                                                                                                                                                                                                                                                                                                 SELECT unnest(ARRAY[439889519552299009]) AS filter_id2                                                                                                                                                                                                                                                                                                                  ) AS uq                                                                                                                                                                                                                                                                                                                                                                     JOIN (                                                                                                                                                                                                                                                                                                                                                                          SELECT unnest(ARRAY[439889519552299009]) AS filter_id                                                                                                                                                                                                                                                                                                                   ) AS ab                                                                                                                                                                                                                                                                                                                                                                     ON uq.filter_id2 = ab.filter_id;
pq: column "uq.filter_id2" does not exist
root@127.0.0.1:58579/defaultdb> set optimizer=off;
SET

Time: 447µs

root@127.0.0.1:58579/defaultdb> SELECT * FROM (                                                                                                                                                                                                                                                                                                                                 SELECT unnest(ARRAY[439889519552299009]) AS filter_id2                                                                                                                                                                                                                                                                                                                  ) AS uq                                                                                                                                                                                                                                                                                                                                                                     JOIN (                                                                                                                                                                                                                                                                                                                                                                          SELECT unnest(ARRAY[439889519552299009]) AS filter_id                                                                                                                                                                                                                                                                                                                   ) AS ab                                                                                                                                                                                                                                                                                                                                                                     ON uq.filter_id2 = ab.filter_id;
      filter_id2     |     filter_id
+--------------------+--------------------+
  439889519552299009 | 439889519552299009
(1 row)

Time: 1.516ms

root@127.0.0.1:58579/defaultdb>
@jordanlewis

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

@RaduBerinde could you further triage?

@RaduBerinde

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

I'll take a first look. Here's a smaller repro:

> SELECT * FROM (SELECT unnest(ARRAY[1]) AS lol) AS t WHERE t.lol = 1;
pq: column "t.lol" does not exist

Also:

root@127.70.228.24:38339/defaultdb> SELECT * FROM (SELECT unnest(ARRAY[1]) AS colalias);
  colalias  
+----------+
         1  
(1 row)

root@127.70.228.24:38339/defaultdb> SELECT * FROM (SELECT unnest(ARRAY[1]) AS colalias) AS tablealias;
  tablealias  
+------------+
           1  
(1 row)

Note how the column changed name when it shouldn't have.

I believe the problematic code is here:
https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/optbuilder/select.go#L212

We should only be doing this if the column didn't have an alias. Not entirely sure how to make that distinction at this point though - maybe add a flag to scopeColumn? CC @rytaft in case you have a better idea.

@RaduBerinde RaduBerinde added this to the 19.1 milestone Apr 4, 2019

@RaduBerinde

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

Actually, we shouldn't be renaming it in either case. This is with postgres:

radu=> SELECT * FROM (SELECT unnest(ARRAY[1])) AS tablealias;
 unnest 
--------
      1
(1 row)
@rytaft

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

Yea, I think I just copied the logic from the heuristic planner and didn't check what Postgres was doing. Although it sounds like I might have made an error in copying from the heuristic planner, too. Anyway, I think we should just get rid of those lines that you found.

Thanks for investigating!

RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Apr 4, 2019

opt: fix inappropriate SRF column rename
When we use a single-column SRF as a data source and use a table alias
(e.g. `SELECT * FROM generate_series(1,2) AS a`), the column is
renamed to the table alias.

The optbuilder does exactly this, however it also does it when the
table alias is "indirect", e.g.
`SELECT * FROM (SELECT * FROM generate_series(1,2)) AS a`.

This commit fixes this by keeping track if a scope has a single column
where this renaming is appropriate, instead of peeking inside the
expression we built, which is in direct violation of Andy's No-peeking
Principle™.

Fixes cockroachdb#36501.

Release note (bug fix): Fixed inappropriate column renaming in some
cases involving single-column SRFs.

RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Apr 4, 2019

opt: fix inappropriate SRF column rename
When we use a single-column SRF as a data source and use a table alias
(e.g. `SELECT * FROM generate_series(1,2) AS a`), the column is
renamed to the table alias.

The optbuilder does exactly this, however it also does it when the
table alias is "indirect", e.g.
`SELECT * FROM (SELECT * FROM generate_series(1,2)) AS a`.

This commit fixes this by keeping track if a scope has a single column
where this renaming is appropriate, instead of peeking inside the
expression we built, which is in direct violation of Andy's No-peeking
Principle™.

Fixes cockroachdb#36501.

Release note (bug fix): Fixed inappropriate column renaming in some
cases involving single-column SRFs.

RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Apr 4, 2019

opt: fix inappropriate SRF column rename
When we use a single-column SRF as a data source and use a table alias
(e.g. `SELECT * FROM generate_series(1,2) AS a`), the column is
renamed to the table alias.

The optbuilder does exactly this, however it also does it when the
table alias is "indirect", e.g.
`SELECT * FROM (SELECT * FROM generate_series(1,2)) AS a`.

This commit fixes this by keeping track if a scope has a single column
where this renaming is appropriate, instead of peeking inside the
expression we built, which is in direct violation of Andy's No-peeking
Principle™.

Fixes cockroachdb#36501.

Release note (bug fix): Fixed inappropriate column renaming in some
cases involving single-column SRFs.
@RaduBerinde

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

A workaround is to move the column alias with the table alias:

SELECT * FROM
  (SELECT unnest(ARRAY[439889519552299009])) AS uq (filter_id2)
JOIN
  (SELECT unnest(ARRAY[439889519552299009])) AS ab (filter_id)
ON uq.filter_id2 = ab.filter_id
@jonwalch

This comment has been minimized.

Copy link

commented Apr 4, 2019

Huge props for moving so fast on this. When can we expect this to make it into a release?

@RaduBerinde

This comment has been minimized.

Copy link
Member

commented Apr 4, 2019

It will be fixed in 19.1 which is coming very soon. Would you need it in a 2.1.x update as well or do you plan to upgrade to 19.1? (the next 2.1.x will likely be out after 19.1)

craig bot pushed a commit that referenced this issue Apr 4, 2019

Merge #36534
36534: opt: fix inappropriate SRF column rename r=RaduBerinde a=RaduBerinde

When we use a single-column SRF as a data source and use a table alias
(e.g. `SELECT * FROM generate_series(1,2) AS a`), the column is
renamed to the table alias.

The optbuilder does exactly this, however it also does it when the
table alias is "indirect", e.g.
`SELECT * FROM (SELECT * FROM generate_series(1,2)) AS a`.

This commit fixes this by keeping track if a scope has a single column
where this renaming is appropriate, instead of peeking inside the
expression we built, which is in direct violation of Andy's No-peeking
Principle™.

Fixes #36501.

Release note (bug fix): Fixed inappropriate column renaming in some
cases involving single-column SRFs.

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>

RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Apr 4, 2019

opt: fix inappropriate SRF column rename
When we use a single-column SRF as a data source and use a table alias
(e.g. `SELECT * FROM generate_series(1,2) AS a`), the column is
renamed to the table alias.

The optbuilder does exactly this, however it also does it when the
table alias is "indirect", e.g.
`SELECT * FROM (SELECT * FROM generate_series(1,2)) AS a`.

This commit fixes this by keeping track if a scope has a single column
where this renaming is appropriate, instead of peeking inside the
expression we built, which is in direct violation of Andy's No-peeking
Principle™.

Fixes cockroachdb#36501.

Release note (bug fix): Fixed inappropriate column renaming in some
cases involving single-column SRFs.
@jonwalch

This comment has been minimized.

Copy link

commented Apr 4, 2019

@RaduBerinde whatever works best for you, we'll use the workaround in the meantime

@craig craig bot closed this in #36534 Apr 4, 2019

SQL Planning automation moved this from To do to Done Apr 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.