cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.91k stars 3.78k forks source link

sql: support attribute notation for single-param functions (x.f equivalent to f(x)) #47477

Open rafiss opened 4 years ago

rafiss commented 4 years ago

This came up during development of the ActiveRecord adapter for CockroachDB. Part of the tool relies on this Postgres behavior. (It's not critical to functionality, but just means some queries may not work.)

rafiss@127:postgres> create table tab (pk int primary key, s varchar, i int);
CREATE TABLE
rafiss@127:postgres> insert into tab values (1, 'a', 1), (2, 'a', 3), (3, 'b', 5);
INSERT 0 3
rafiss@127:postgres> select s, max(i) from tab group by s order by tab.count desc;
+-----+-------+
| s   | max   |
|-----+-------|
| a   | 3     |
| b   | 5     |
+-----+-------+

In CockroachDB, the above results in ERROR: column "tab.count" does not exist

Here is the ActiveRecord test

  def test_group_by_with_order_by_virtual_count_attribute
    expected = { "SpecialPost" => 1, "StiPost" => 2 }
    actual = Post.group(:type).order(:count).limit(2).maximum(:comments_count)
    assert_equal expected, actual
  end if current_adapter?(:PostgreSQLAdapter)

Jira issue: CRDB-4406

rytaft commented 4 years ago

cc @RaduBerinde, @awoods187

RaduBerinde commented 4 years ago

It looks like this is some special syntax that allows interpreting x.f as f(x), combined with the fact that postgres allows count(table) (as opposed to count(table.*)).

AFAICT it is always equivalent to use count(*) or count(t.*). I this case I believe the code could just do .order("COUNT(*)") instead of .order(:count).

I would push back against implementing this unless the workaround I mentioned doesn't work or we have a strong reason to support existing code that does this.

rafiss commented 4 years ago

This section of the Postgres docs covers the behavior that Radu describes: https://www.postgresql.org/docs/12/rowtypes.html#ROWTYPES-USAGE

I'll rename the issue to make it clear it's more of a syntax thing, and not actually a new column.

And some other relevant reading: https://dba.stackexchange.com/questions/129410/order-by-count-and-bitmap-heap-scan https://stackoverflow.com/questions/11165450/store-common-query-as-column/11166268#11166268

mgartner commented 2 years ago

@vy-ton I'm moving to the backlog, but if ActiveRecord is a high priority for 22.2, then let me know so I can reprioritize this.

github-actions[bot] commented 10 months ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!