8000 Parse and represent window functions in the LQP by dey4ss · Pull Request #2574 · hyrise/hyrise · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Parse and represent window fun 8000 ctions in the LQP #2574

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

Merged
merged 32 commits into from
Jul 26, 2023

Conversation

dey4ss
Copy link
Member
@dey4ss dey4ss commented Jun 8, 2023

@Bouncner you want it, you get it 🖤

This PR adds the representation of SQL:2003 window functions in the LQP. This includes the translation in the SQLTranslator.

Changes are:

  1. AggregateExpression is now WindowFunctionExpression. It uses one argument for the WindowExpression that defines the window. If it does not reference a window and the function is allowed for aggregates (sum, min, avg, etc.), it is a regular aggregate expression.
  2. WindowExpression is an expression describing the window (tests included).
  3. WindowNode is the LQPNode representing the to-be-implemented window operator (tests included).
  4. SQLTranslator translates window functions correctly (tests included) and rejects invalid queries.
  5. Minimal adaptions to AggregateNode, ColumnPruningRule, and CardinalityEstimator to handle WindowNodes and window functions (tests included).
  6. AggregateTraits are now WindowFunctionTraits. Return types of additional window functions are added.
  7. Automated renaming necessary due to 1. and 6.
  8. (minor) hyriseConsole now also catches InvalidInputExceotions that are thrown when a query is passed directly to the visualize command.

Note that this is branched off #2571, so this PR currently also includes its diff.

@dey4ss dey4ss added Feature FullCI Run all CI tests (slow, but required for merge) labels Jun 8, 2023
Copy link
Collaborator
@Bouncner Bouncner left a comment

Choose a reason for hiding this comment

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

Haven't looked at the tests yet.

@hyrise hyrise deleted a comment from dey4ss Jun 30, 2023
Copy link
Collaborator
@Bouncner Bouncner left a comment

Choose a reason for hiding this comment

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

Remainder.

@@ -289,6 +292,27 @@ std::shared_ptr<TableStatistics> CardinalityEstimator::estimate_projection_node(
return std::make_shared<TableStatistics>(std::move(column_statistics), input_table_statistics->row_count);
}

std::shared_ptr<TableStatistics> CardinalityEstimator::estimate_window_node(
const WindowNode& window_node, const std::shared_ptr<TableStatistics>& input_table_statistics) const {
// For the result of the window function, dummy statistics are created for now.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Never said they are always going to be distinct. I thought about a fair assumption for cardinality estimations. In most cases, it will be distinct (I'd guess) and in other cases, we do at least not underestimate. But nevermind ... probably not relevant for now.

// NOLINTNEXTLINE - while this particular method could be made static, others cannot.
std::shared_ptr<AbstractOperator> LQPTranslator::_translate_window_node(
const std::shared_ptr<AbstractLQPNode>& node) const {
FailInput("Hyrise does not yet support window functions.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing for this PR, yes. But I wanted to note it somewhere to discuss it after the vacations.

Copy link
Collaborator
@Bouncner Bouncner left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dey4ss dey4ss merged commit b685721 into master Jul 26, 2023
@dey4ss dey4ss deleted the dey4ss/prepare_dyod23_window branch July 26, 2023 08:43
@Bouncner Bouncner mentioned this pull request Aug 9, 2023
16 tasks
nikriek pushed a commit that referenced this pull request Oct 28, 2023
Add the representation of SQL:2003 window functions in the LQP, including the translation in the SQLTranslator.
nikriek pushed a commit that referenced this pull request Nov 6, 2023
Add the representation of SQL:2003 window functions in the LQP, including the translation in the SQLTranslator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature FullCI Run all CI tests (slow, but required for merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0