Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-207] fix unsuppored types in aggregate #299

Merged
merged 2 commits into from
May 26, 2021

Conversation

rui-mo
Copy link
Collaborator

@rui-mo rui-mo commented May 8, 2021

What changes were proposed in this pull request?

This pr fixed the unsuppored types in aggregate.

#207

How was this patch tested?

verified on TPC-DS v2, TPC-DS v1

@github-actions
Copy link

github-actions bot commented May 8, 2021

#207

@zhouyuan
Copy link
Collaborator

@rui-mo C++ unit tests need to be refreshed?

@@ -1977,6 +2316,183 @@ class AvgAction : public ActionBase {
uint64_t length_ = 0;
};

// Decimal
template <typename DataType, typename CType, typename ResDataType, typename ResCType>
class AvgAction<DataType, CType, ResDataType, ResCType,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is AVG on decimal column a valid case? seems on Spark it will cast decimal to long first

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like sum could be decimal, count is long type. And average could be decimal type.

  private lazy val resultType = child.dataType match {
    case DecimalType.Fixed(p, s) =>
      DecimalType.bounded(p + 4, s + 4)
    case _ => DoubleType
  }

  private lazy val sumDataType = child.dataType match {
    case _ @ DecimalType.Fixed(p, s) => DecimalType.bounded(p + 10, s)
    case _ => DoubleType
  }

  private lazy val sum = AttributeReference("sum", sumDataType)()
  private lazy val count = AttributeReference("count", LongType)()

@zhouyuan zhouyuan merged commit 2db84f7 into oap-project:master May 26, 2021
@rui-mo rui-mo deleted the fix_v2 branch July 13, 2021 08:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants