facebookincubator / velox

A composable and fully extensible C++ execution engine library for data management systems.
https://velox-lib.io/
Apache License 2.0
3.52k stars 1.15k forks source link

SelectiveDecimalColumnReader::scanSpec_::valueHook_ may not be nullptr #11290

Closed NEUpanning closed 2 days ago

NEUpanning commented 4 weeks ago

Bug description

Exception is caused by SelectiveDecimalColumnReader::scanSpec_::valueHook_ != nullptr when the physical type of decimal is int64_t and aggregation push-down is enabled.

Error Source: RUNTIME
Error Code: INVALID_STATE
Retriable: False
Expression: !scanSpec_->valueHook()
Context: Split [Hive: viewfs://hadoop-meituan/ghann03/warehouse/app_hotel.db/app_poi_leopard_coverage_medal/datekey=20240910/000000_0 0 - 63768834] Task Gluten_Stage_0_TID_31_VTID_0
Additional Context: Operator: PartialAggregation[1] 1
Function: read
File: /opt/meituan/panning/gluten-dev/velox/velox/dwio/dwrf/reader/SelectiveDecimalColumnReader.cpp
Line: 115
facebook::velox::process::StackTrace::StackTrace(int)
_ZN8facebook5velox14VeloxException5State4makeIZNS1_C4EPKcmS5_St17basic_string_viewIcSt11char_traitsIcEES9_S9_S9_bNS1_4TypeES9_EUlRT_E_EESt10shared_ptrIKS2_ESA_SB_
facebook::velox::VeloxException::VeloxException(char const*, unsigned long, char const*, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, bool, facebook::velox::VeloxException::Type, std::basic_string_view<char, std::char_traits<char> >)
facebook::velox::VeloxRuntimeError::VeloxRuntimeError(char const*, unsigned long, char const*, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, std::basic_string_view<char, std::char_traits<char> >, bool, std::basic_string_view<char, std::char_traits<char> >)
void facebook::velox::detail::veloxCheckFail<facebook::velox::VeloxRuntimeError, facebook::velox::detail::CompileTimeEmptyString>(facebook::velox::detail::VeloxCheckFailArgs const&, facebook::velox::detail::CompileTimeEmptyString)
facebook::velox::dwrf::SelectiveDecimalColumnReader<long>::read(int, folly::Range<int const*>, unsigned long const*)
facebook::velox::dwio::common::ColumnLoader::loadInternal(folly::Range<int const*>, facebook::velox::ValueHook*, int, std::shared_ptr<facebook::velox::BaseVector>*)
facebook::velox::VectorLoader::load(folly::Range<int const*>, facebook::velox::ValueHook*, int, std::shared_ptr<facebook::velox::BaseVector>*)
facebook::velox::LazyVector::load(folly::Range<int const*>, facebook::velox::ValueHook*) const
void facebook::velox::functions::aggregate::SimpleNumericAggregate<long, long, long>::pushdown<facebook::velox::aggregate::MinMaxHook<long, false> >(char**, facebook::velox::SelectivityVector const&, std::shared_ptr<facebook::velox::BaseVector> const&)
facebook::velox::functions::aggregate::(anonymous namespace)::MaxAggregate<long>::addRawInput(char**, facebook::velox::SelectivityVector const&, std::vector<std::shared_ptr<facebook::velox::BaseVector>, std::allocator<std::shared_ptr<facebook::velox::BaseVector> > > const&, bool)
facebook::velox::exec::AggregateCompanionFunctionBase::addRawInput(char**, facebook::velox::SelectivityVector const&, std::vector<std::shared_ptr<facebook::velox::BaseVector>, std::allocator<std::shared_ptr<facebook::velox::BaseVector> > > const&, bool)
facebook::velox::exec::GroupingSet::addInputForActiveRows(std::shared_ptr<facebook::velox::RowVector> const&, bool)
facebook::velox::exec::GroupingSet::addInput(std::shared_ptr<facebook::velox::RowVector> const&, bool)
facebook::velox::exec::HashAggregation::addInput(std::shared_ptr<facebook::velox::RowVector>)
facebook::velox::exec::Driver::runInternal(std::shared_ptr<facebook::velox::exec::Driver>&, std::shared_ptr<facebook::velox::exec::BlockingState>&, std::shared_ptr<facebook::velox::RowVector>&)
facebook::velox::exec::Driver::next(std::shared_ptr<facebook::velox::exec::BlockingState>&)
facebook::velox::exec::Task::next(folly::SemiFuture<folly::Unit>*)
gluten::WholeStageResultIterator::next()
gluten::ResultIterator::getNext()
gluten::ResultIterator::hasNext()

I see that currently only int128_t doesn't enable push-down for SelectiveDecimalColumnReader, but decimal also can be int64_t. Perhaps, we should let int64_t do this as well.

System information

Velox System Info v0.0.2 Commit: 33928e00 CMake Version: 3.29.6 System: Linux-3.10.0-862.mt20190308.130.el7.x86_64 Arch: x86_64 C++ Compiler: /opt/rh/devtoolset-10/root/usr/bin/c++ C++ Compiler Version: 10.2.1 C Compiler: /opt/rh/devtoolset-10/root/usr/bin/cc C Compiler Version: 10.2.1

Relevant logs

No response

Yuhta commented 4 weeks ago

int64_t needs to be enabled, we need to check in the aggregate function that !type->isDecimal() to enable pushdowns.

NEUpanning commented 4 weeks ago

@Yuhta Thanks for your reply. I'd like to work on it.