Different result vector size between common and simplified path

[kagamiori]

Description

Fuzzer found a bug when evaluating an expression with all input vectors of size 86, the common path produces a result vector of size 86 whereas the simplified path produces a result vector of 100. The reason is that the result vector is moveOrCopyResult-ed from a sharedSubexprValues vector produced by evaluating on inner rows after peeling that have size 100. moveOrCopyResult(localResult, rows, result) in this case does receive a selectivity vector of size 86, but because result is a nullptr, it simply assign localResult to result.

Error Reproduction

The error can be reproduced by checking out D46245166 internally with the attached files.

velox_expressionVerifier_7l561q.zip

Relevant logs

I0526 20:30:39.869194 1352861 ExpressionRunner.cpp:184] Evaluating SQL expression(s): "try"("switch"("subscript"("c0", "array_min"("c1")), "is_null"('6338838335202944843'::BIGINT), ("c2") and ("is_null"('6338838335202944843'::BIGINT)) and (TRUE) and ("gte"(NULL::SMALLINT, "c3")) and ("coalesce"("c4", "c5", FALSE)), FALSE, "is_null"('6338838335202944843'::BIGINT)))
I0526 20:30:39.869648 1352861 ExpressionVerifier.cpp:80] Executing expression 0 : try(try(switch(subscript(ROW["c0"],array_min(ROW["c1"])),is_null(cast "6338838335202944843" as BIGINT),and(and(and(and(ROW["c2"],is_null(cast "6338838335202944843" as BIGINT)),true),gte(cast null as SMALLINT,ROW["c3"])),coalesce(ROW["c4"],ROW["c5"],false)),false,is_null(cast "6338838335202944843" as BIGINT))))
I0526 20:30:39.870245 1352861 ExpressionVerifier.cpp:31] 6 vectors as input:
I0526 20:30:39.870292 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY ARRAY<BOOLEAN>: 100 elements, 8 nulls], [ARRAY ARRAY<BOOLEAN>: 100 elements, no nulls]
I0526 20:30:39.870465 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY ARRAY<BIGINT>: 100 elements, 11 nulls], [ARRAY ARRAY<BIGINT>: 100 elements, no nulls]
I0526 20:30:39.870541 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY BOOLEAN: 100 elements, 3 nulls], [DICTIONARY BOOLEAN: 100 elements, 18 nulls], [FLAT BOOLEAN: 100 elements, 9 nulls]
I0526 20:30:39.870636 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY SMALLINT: 100 elements, 4 nulls], [DICTIONARY SMALLINT: 100 elements, 11 nulls], [CONSTANT SMALLINT: 100 elements, 11184]
I0526 20:30:39.870731 1352861 ExpressionVerifier.cpp:33] 	[CONSTANT BOOLEAN: 100 elements, false]
I0526 20:30:39.870867 1352861 ExpressionVerifier.cpp:33] 	[FLAT BOOLEAN: 100 elements, 11 nulls]
E0526 20:30:39.876892 1352861 Exceptions.h:68] Line: buck-out/dev/gen/aab7ed39/velox/functions/lib/velox_functions_lib#header-mode-symlink-tree-with-header-map,headers/velox/functions/lib/SubscriptUtil.h:284, Function:getIndex, Expression:  Array subscript out of bounds., Source: USER, ErrorCode: INVALID_ARGUMENT
...
I0526 20:30:39.891021 1352861 ExpressionVerifier.cpp:68] All results match.
I0526 20:30:39.891491 1352861 ExpressionRunner.cpp:220] Retrying original expression on 86 rows without errors
I0526 20:30:39.891734 1352861 ExpressionVerifier.cpp:80] Executing expression 0 : try(switch(subscript(ROW["c0"],array_min(ROW["c1"])),is_null(cast "6338838335202944843" as BIGINT),and(and(and(and(ROW["c2"],is_null(cast "6338838335202944843" as BIGINT)),true),gte(cast null as SMALLINT,ROW["c3"])),coalesce(ROW["c4"],ROW["c5"],false)),false,is_null(cast "6338838335202944843" as BIGINT)))
I0526 20:30:39.892079 1352861 ExpressionVerifier.cpp:31] 6 vectors as input:
I0526 20:30:39.892113 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY ARRAY<BOOLEAN>: 86 elements, no nulls], [DICTIONARY ARRAY<BOOLEAN>: 100 elements, 8 nulls], [ARRAY ARRAY<BOOLEAN>: 100 elements, no nulls]
I0526 20:30:39.892210 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY ARRAY<BIGINT>: 86 elements, no nulls], [DICTIONARY ARRAY<BIGINT>: 100 elements, 11 nulls], [ARRAY ARRAY<BIGINT>: 100 elements, no nulls]
I0526 20:30:39.892282 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY BOOLEAN: 86 elements, no nulls], [DICTIONARY BOOLEAN: 100 elements, 3 nulls], [DICTIONARY BOOLEAN: 100 elements, 18 nulls], [FLAT BOOLEAN: 100 elements, 9 nulls]
I0526 20:30:39.892390 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY SMALLINT: 86 elements, no nulls], [DICTIONARY SMALLINT: 100 elements, 4 nulls], [DICTIONARY SMALLINT: 100 elements, 11 nulls], [CONSTANT SMALLINT: 100 elements, 11184]
I0526 20:30:39.892473 1352861 ExpressionVerifier.cpp:33] 	[CONSTANT BOOLEAN: 86 elements, false]
I0526 20:30:39.892544 1352861 ExpressionVerifier.cpp:33] 	[DICTIONARY BOOLEAN: 86 elements, no nulls], [FLAT BOOLEAN: 100 elements, 11 nulls]
E0526 20:30:39.901437 1352861 Exceptions.h:68] Line: velox/expression/tests/ExpressionVerifier.cpp:47, Function:compareVectors, Expression: left->size() == right->size() (86 vs. 100), Source: RUNTIME, ErrorCode: INVALID_STATE
I0526 20:30:39.902181 1352861 ExpressionVerifier.cpp:232] Skipping persistence because repro path is empty.
terminate called after throwing an instance of 'facebook::velox::VeloxRuntimeError'
  what():  Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: INVALID_STATE
Reason: (86 vs. 100)
Retriable: False
Expression: left->size() == right->size()
Function: compareVectors
File: velox/expression/tests/ExpressionVerifier.cpp
Line: 47
Stack trace:
# 0  std::shared_ptr<facebook::velox::VeloxException::State const> facebook::velox::VeloxException::State::make<facebook::velox::VeloxException::make(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> >)::$_0>(facebook::velox::VeloxException::Type, facebook::velox::VeloxException::make(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> >)::$_0)
# 1  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> >)
# 2  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> >)
# 3  void facebook::velox::detail::veloxCheckFail<facebook::velox::VeloxRuntimeError, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(facebook::velox::detail::VeloxCheckFailArgs const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
# 4  facebook::velox::test::(anonymous namespace)::compareVectors(std::shared_ptr<facebook::velox::BaseVector> const&, std::shared_ptr<facebook::velox::BaseVector> const&)
# 5  facebook::velox::test::ExpressionVerifier::verify(std::vector<std::shared_ptr<facebook::velox::core::ITypedExpr const>, std::allocator<std::shared_ptr<facebook::velox::core::ITypedExpr const> > > const&, std::shared_ptr<facebook::velox::RowVector> const&, std::shared_ptr<facebook::velox::BaseVector>&&, bool, std::vector<unsigned int, std::allocator<unsigned int> >)
# 6  facebook::velox::test::ExpressionRunner::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
# 7  main
# 8  __libc_start_call_main
# 9  __libc_start_main_alias_2
# 10 _start

*** Aborted at 1685158239 (Unix time, try 'date -d @1685158239') ***
*** Signal 6 (SIGABRT) (0x37b3c0014a49d) received by PID 1352861 (pthread TID 0x7fc7e5852100) (linux TID 1352861) (maybe from PID 1352861, UID 228156) (code: -6), stack trace: ***
    @ 0000000000010fae folly::symbolizer::(anonymous namespace)::innerSignalHandler(int, siginfo_t*, void*)
                       ./folly/experimental/symbolizer/SignalHandler.cpp:449
    @ 000000000000f6d1 folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       ./folly/experimental/symbolizer/SignalHandler.cpp:470
    @ 000000000004459f (unknown)
    @ 000000000009c9d3 __GI___pthread_kill
    @ 00000000000444ec __GI_raise
    @ 000000000002c432 __GI_abort
    @ 00000000000a3fd4 __gnu_cxx::__verbose_terminate_handler()
    @ 00000000000a1b39 __cxxabiv1::__terminate(void (*)())
    @ 00000000000a1ba4 std::terminate()
    @ 00000000000a1ec1 __cxa_rethrow
    @ 00000000000261cd facebook::velox::test::ExpressionVerifier::verify(std::vector<std::shared_ptr<facebook::velox::core::ITypedExpr const>, std::allocator<std::shared_ptr<facebook::velox::core::ITypedExpr const> > > const&, std::shared_ptr<facebook::velox::RowVector> const&, std::shared_ptr<facebook::velox::BaseVector>&&, bool, std::vector<unsigned int, std::allocator<unsigned int> >)
                       ./velox/expression/tests/ExpressionVerifier.cpp:206
    @ 000000000002f34c facebook::velox::test::ExpressionRunner::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
                       ./velox/expression/tests/ExpressionRunner.cpp:223
    @ 00000000002cd296 main
                       ./velox/expression/tests/ExpressionRunnerTest.cpp:182
    @ 000000000002c656 __libc_start_call_main
    @ 000000000002c717 __libc_start_main_alias_2
    @ 00000000002b81d0 _start
                       /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/x86_64/start.S:116
Aborted (core dumped)

[laithsakka]

simplified SQL to :

"try"(
"switch"(
"subscript"("c0", "array_min"("c1")),
FALSE,
FALSE,
FALSE,
"is_null"('6338838335202944843'::BIGINT)))

[laithsakka]

the problem is that the expression above have propagatesNulls_ true, however for null input switch takes the else branch and hence switch is not null propagating. and the expresion

"try"(
"switch"(
"subscript"("c0", "array_min"("c1")),
FALSE,
FALSE,
FALSE,
"is_null"('6338838335202944843'::BIGINT)))

should have null propagate = false.

[laithsakka]

same as https://docs.google.com/document/d/1H3fCIe7v4FvEGAtUo3pz5OK5BAQTaR9LgI40Vh24IOU/edit#