Skip to content

Fix invalid 'rawResultNulls_' in SelectiveDecimalColumnReader#10925

Closed
rui-mo wants to merge 1 commit intofacebookincubator:mainfrom
rui-mo:wip_dec_null
Closed

Fix invalid 'rawResultNulls_' in SelectiveDecimalColumnReader#10925
rui-mo wants to merge 1 commit intofacebookincubator:mainfrom
rui-mo:wip_dec_null

Conversation

@rui-mo
Copy link
Copy Markdown
Contributor

@rui-mo rui-mo commented Sep 4, 2024

Fixes exception caused by invalid 'rawResultNulls_' when reading decimal column
containing null value from ORC file.

velox/dwio/common/SelectiveColumnReader.h:326, Function:addNull, Expression: rawResultNulls_ && rawValues_ , Source: RUNTIME, ErrorCode: INVALID_STATE
unknown file: Failure
C++ exception with description "Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: INVALID_STATE
Retriable: False
Expression: rawResultNulls_ && rawValues_

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 4, 2024
@netlify
Copy link
Copy Markdown

netlify Bot commented Sep 4, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 2353a79
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66d8101cec74000008633ae0

VELOX_CHECK(!scanSpec_->filter());
VELOX_CHECK(!scanSpec_->valueHook());
prepareRead<int64_t>(offset, rows, incomingNulls);
if (!resultNulls_ || !resultNulls_->unique() ||
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should not need to handle this in column reader subclass like this. Do you have the full stack trace of the exception? resultNulls_ should be kept unallocated only if we can reuse incoming nulls.

Copy link
Copy Markdown
Contributor Author

@rui-mo rui-mo Sep 5, 2024

Choose a reason for hiding this comment

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

Below is the full stack trace for this exception. Thanks.

Details
# 3  void facebook::velox::detail::veloxCheckFail<facebook::velox::VeloxRuntimeError, facebook::velox::detail::CompileTimeEmptyString>(facebook::velox::detail::VeloxCheckFailArgs const&, facebook::velox::detail::CompileTimeEmptyString)
# 4  void facebook::velox::dwio::common::SelectiveColumnReader::addNull<long>()
# 5  void facebook::velox::dwio::common::ExtractToReader::addNull<long>(int)
# 6  facebook::velox::dwio::common::ColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true>::addNull()
# 7  facebook::velox::dwio::common::ColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true>::filterPassedForNull()
# 8  facebook::velox::dwio::common::ColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true>::processNull(bool&)
# 9  void facebook::velox::dwrf::RleDecoderV2<true>::readWithVisitor<true, facebook::velox::dwio::common::DirectRleColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true> >(unsigned long const*, facebook::velox::dwio::common::DirectRleColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true>)
# 10 void facebook::velox::dwio::common::SelectiveColumnReader::decodeWithVisitor<facebook::velox::dwrf::RleDecoderV2<true>, facebook::velox::dwio::common::DirectRleColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true> >(facebook::velox::dwio::common::IntDecoder<facebook::velox::dwrf::RleDecoderV2<true>::kIsSigned>*, facebook::velox::dwio::common::DirectRleColumnVisitor<long, facebook::velox::common::AlwaysTrue, facebook::velox::dwio::common::ExtractToReader, true>&)
# 11 void facebook::velox::dwrf::SelectiveDecimalColumnReader<long>::readHelper<true>(folly::Range<int const*>)
# 12 facebook::velox::dwrf::SelectiveDecimalColumnReader<long>::read(int, folly::Range<int const*>, unsigned long const*)
# 13 facebook::velox::dwio::common::ColumnLoader::loadInternal(folly::Range<int const*>, facebook::velox::ValueHook*, int, std::shared_ptr<facebook::velox::BaseVector>*)
# 14 facebook::velox::VectorLoader::load(folly::Range<int const*>, facebook::velox::ValueHook*, int, std::shared_ptr<facebook::velox::BaseVector>*)
# 15 facebook::velox::LazyVector::load(folly::Range<int const*>, facebook::velox::ValueHook*) const
# 16 facebook::velox::LazyVector::ensureLoadedRowsImpl(std::shared_ptr<facebook::velox::BaseVector> const&, facebook::velox::DecodedVector&, facebook::velox::SelectivityVector const&, facebook::velox::SelectivityVector&)
# 17 facebook::velox::LazyVector::ensureLoadedRows(std::shared_ptr<facebook::velox::BaseVector> const&, facebook::velox::SelectivityVector const&, facebook::velox::DecodedVector&, facebook::velox::SelectivityVector&)
# 18 facebook::velox::exec::loadColumns(std::shared_ptr<facebook::velox::RowVector> const&, facebook::velox::core::ExecCtx&)
# 19 facebook::velox::exec::CallbackSink::addInput(std::shared_ptr<facebook::velox::RowVector>)
# 20 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>&)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be fixed in #10939

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the pointer. Indeed it has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants