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,opt: composite_value and composite_value.* should be identical in some contexts (e.g. as a function argument) #60549

Closed
rafiss opened this issue Feb 12, 2021 · 2 comments · Fixed by #64748
Assignees
Labels
A-sql-builtins SQL built-in functions and semantics thereof. A-sql-name-resolution SQL name resolution rules and CTEs. A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-graphile Issues relating to graphile compatibility C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@rafiss
Copy link
Collaborator

rafiss commented Feb 12, 2021

The syntax select row_to_json(x) from my_table x is not supported. CockroachDB assumes x is a column rather than a table reference. A workaround is to use select row_to_json(x.*) from my_table x.

Tools like PostGraphile use this. See graphile/crystal#645 (comment)

The PostgreSQL docs explain more: https://www.postgresql.org/docs/13/rowtypes.html

The composite_value.* syntax results in column expansion of this kind when it appears at the top level of a SELECT output list, a RETURNING list in INSERT/UPDATE/DELETE, a VALUES clause, or a row constructor. In all other contexts (including when nested inside one of those constructs), attaching .* to a composite value does not change the value, since it means “all columns” and so the same composite value is produced again. For example, if somefunc() accepts a composite-valued argument, these queries are the same:

SELECT somefunc(c.*) FROM inventory_item c;
SELECT somefunc(c) FROM inventory_item c;

In both cases, the current row of inventory_item is passed to the function as a single composite-valued argument. Even though .* does nothing in such cases, using it is good style, since it makes clear that a composite value is intended. In particular, the parser will consider c in c.* to refer to a table name or alias, not to a column name, so that there is no ambiguity; whereas without .*, it is not clear whether c means a table name or a column name, and in fact the column-name interpretation will be preferred if there is a column named c.

It seems like we need to update

case *tree.UnresolvedName:
vn, err := t.NormalizeVarName()
if err != nil {
panic(err)
}
return s.VisitPre(vn)

Right now, t.NormalizeVarName returns either a tree.AllColumnsSelector or a tree.ColumnItem. Currently it decides that based solely on if there is a star expansion in the expression. Perhaps it should be updated to return tree.AllColumnsSelector according to the rules above.

Relates to:

@rafiss rafiss added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-pgcompat Semantic compatibility with PostgreSQL A-sql-builtins SQL built-in functions and semantics thereof. labels Feb 12, 2021
@rafiss rafiss added A-sql-name-resolution SQL name resolution rules and CTEs. A-sql-json JSON handling in SQL. labels Feb 17, 2021
@rafiss rafiss changed the title sql: support row_to_json(x) sql,opt: composite_value and composite_value.* should be identical in some contexts (e.g. as a function argument) Feb 17, 2021
@rafiss rafiss removed the A-sql-json JSON handling in SQL. label Feb 17, 2021
@rafiss
Copy link
Collaborator Author

rafiss commented Feb 17, 2021

I've renamed this issue since it's not really specific to row_to_json.

cc @RaduBerinde to triage / determine feasibility.

@rafiss
Copy link
Collaborator Author

rafiss commented Feb 17, 2021

Further complicated by the case where table a also has a column a:

Postgres:

rafiss@127:postgres> \d a
+----------+---------+-------------+
| Column   | Type    | Modifiers   |
|----------+---------+-------------|
| a        | integer |  not null   |
| b        | integer |             |
+----------+---------+-------------+
Indexes:
    "a_pkey" PRIMARY KEY, btree (a)


rafiss@127:postgres> select row_to_json(a) from a limit 1;
2021-02-17 13:18:32.571 EST [21919] ERROR:  function row_to_json(integer) does not exist at character 8
2021-02-17 13:18:32.571 EST [21919] HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
2021-02-17 13:18:32.571 EST [21919] STATEMENT:  select row_to_json(a) from a limit 1
function row_to_json(integer) does not exist
LINE 1: select row_to_json(a) from a limit 1
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.


rafiss@127:postgres> select row_to_json(a_alias) from a a_alias limit 1;
+----------------+
| row_to_json    |
|----------------|
| {"a":1,"b":10} |
+----------------+

@rafiss rafiss added the A-tools-graphile Issues relating to graphile compatibility label Mar 15, 2021
@exalate-issue-sync exalate-issue-sync bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 3, 2021
@craig craig bot closed this as completed in 9fed2b5 May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-builtins SQL built-in functions and semantics thereof. A-sql-name-resolution SQL name resolution rules and CTEs. A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-graphile Issues relating to graphile compatibility C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants