facebookincubator / velox

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

Only simplified path threw exception: IN predicate supports only constant IN list #3041

Open mbasmanova opened 2 years ago

mbasmanova commented 2 years ago
I1031 18:22:21.418013 1281475 ExpressionFuzzer.cpp:702] ==============================> Started iteration 169 (seed: 455940635)
I1031 18:22:21.419494 1281475 ExpressionVerifier.cpp:141] Executing expression: in(-40,reverse(10 elements starting at 470 {[470->110] null, null, [472->426] [426->147] 49, [473->461] [46
1->188] 113, [474->144] [144->331] null, ...}))
I1031 18:22:21.419672 1281475 ExpressionVerifier.cpp:144] 0 vectors as input:
E1031 18:22:21.423998 1281475 Exceptions.h:68] Line: velox/functions/prestosql/InPredicate.cpp:218, Function:applyTyped, Expression: filter_ IN predicate supports only constant IN list, S
ource: RUNTIME, ErrorCode: INVALID_STATE
E1031 18:22:21.424413 1281475 ExpressionVerifier.cpp:75] Only simplified path threw exception:
I1031 18:22:21.425695 1281475 ExpressionVerifier.cpp:309] Persisted input: --input_path /tmp/a/velox_vector_d7QDdi --result_path /tmp/a/velox_vector_yKhEYC --sql_path /tmp/a/velox_sql_ZWd
ekd
terminate called after throwing an instance of 'facebook::velox::VeloxRuntimeError'
  what():  Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: INVALID_STATE
Reason: IN predicate supports only constant IN list
Retriable: False
Expression: filter_
Function: applyTyped
File: velox/functions/prestosql/InPredicate.cpp
Line: 218

Repro files:

in.tar.gz

kagamiori commented 2 years ago

The evaluated expression is in(-40, reverse(ARRAY [...])). The common evaluation path constant-folds the second sub-expression such that when in is evaluated, InPredicate::create() sees the second argument being a constant. On the other hand, the simplified evaluation path does not do constant-folding, so InPredicate::create() sees the second argument being an expression. But it requires the second argument being a constant and hence throws.

I feel both behaviors are expected respectively.

kagamiori commented 1 year ago

@oerling is going to enable constant-folding in simplified path as well. This change will make the two paths behave the same.

mbasmanova commented 1 year ago

@kagamiori @oerling Is there an ETA for the fix?

laithsakka commented 1 year ago

Looking at this, actually the current VELOX implementation of in as function is not accurate.

Velox support select 1 in array(1,2,3) and select 1 in reverse(1,2,3) which are not supported in prestoSQL. I tried the following in presto and they are not allowed:

select 1 in Array[1,2,3] select 1 in reverse(array(1,2,3))

I propose that we right IN as special form that have constant list of literals.