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

Add aliasing to create named tuples #32

Merged
merged 2 commits into from
Oct 9, 2024
Merged

Add aliasing to create named tuples #32

merged 2 commits into from
Oct 9, 2024

Conversation

BenoitRanque
Copy link
Collaborator

Our generated SQL casts anonymous tuples to named tuples at the very top:

-- cast the anonymou tuple to a named tuple, relying on position
SELECT cast(
  _rowset._rowset,
  'tuple(foo String, bar String)'
) FROM (
  -- create anonymous, positional tuple
  -- starting in 24.7, creates a named tuple with fields _field_foo and _field_bar
  SELECT tuple(
    _row._field_foo,
    _row._field_bar
  ) AS _rowset
  FROM (
    SELECT
      -- fields aliased with a namespace
      -- required as we may select fields for other reasons
      -- (eg. ordering, relationships) 
      'foo' AS _field_foo,
      'bar' AS _field_bar
  ) AS _row
) AS _rowset

The above fails starting in ClickHouse 24.7 due to a breaking change:

Function tuple will now try to construct named tuples in query (controlled by enable_named_columns_in_function_tuple). Introduce function tupleNames to extract names from tuples.

This manifests is a very weird way: when casting, we end up getting the default value for the data type, because we cannot fetch the correct column in the tuple.

Turns out, it's possible to alias fields when creating a named tuple, to set the names for the tuple:

-- casting now works
SELECT cast(
  _rowset._rowset,
  'tuple(foo String, bar String)'
) FROM (
  -- create a named tuple
  SELECT tuple(
    _row._field_foo as foo,
    _row._field_bar as bar
  ) AS _rowset
  FROM (
    SELECT
      'foo' AS _field_foo,
      'bar' AS _field_bar
  ) AS _row
) AS _rowset

Ident::new_quoted(format!("_agg_{alias}")),
]);
apply_function(&aggregate_function(function)?, column)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This above is just a format change right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mostly formatting yes. there's an actual change at the bottom, and I guess cargo decided that warranted a reformat

@BenoitRanque BenoitRanque merged commit 81000ac into main Oct 9, 2024
13 checks passed
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