Skip to content

Commit f9cac51

Browse files
jinchengchenghhzhejiangxiaomai
authored andcommitted
Fix filter bound in BigintRange (#114)
1 parent c456fed commit f9cac51

File tree

3 files changed

+155
-4
lines changed

3 files changed

+155
-4
lines changed

velox/substrait/tests/Substrait2VeloxPlanConversionTest.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,17 @@ TEST_F(Substrait2VeloxPlanConversionTest, ifthenTest) {
312312
" -- TableScan[table: hive_table, range filters: [(hd_buy_potential, Filter(MultiRange, deterministic, null not allowed)), (hd_demo_sk, Filter(IsNotNull, deterministic, null not allowed)), (hd_vehicle_count, BigintRange: [1, 9223372036854775807] no nulls)], remaining filter: (if(greaterthan(ROW[\"hd_vehicle_count\"],0),greaterthan(divide(cast ROW[\"hd_dep_count\"] as DOUBLE,cast ROW[\"hd_vehicle_count\"] as DOUBLE),1.2)))] -> n0_0:BIGINT, n0_1:VARCHAR, n0_2:BIGINT, n0_3:BIGINT\n",
313313
planNode->toString(true, true));
314314
}
315+
316+
TEST_F(Substrait2VeloxPlanConversionTest, filterUpper) {
317+
std::string subPlanPath =
318+
getDataFilePath("velox/substrait/tests", "data/filter_upper.json");
319+
320+
::substrait::Plan substraitPlan;
321+
JsonToProtoConverter::readFromFile(subPlanPath, substraitPlan);
322+
323+
// Convert to Velox PlanNode.
324+
facebook::velox::substrait::SubstraitVeloxPlanConverter planConverter(
325+
pool_.get());
326+
auto planNode = planConverter.toVeloxPlan(substraitPlan);
327+
328+
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
{
2+
"extensions": [{
3+
"extensionFunction": {
4+
"name": "is_not_null:opt_bool_i32"
5+
}
6+
}, {
7+
"extensionFunction": {
8+
"functionAnchor": 2,
9+
"name": "and:opt_bool_bool"
10+
}
11+
}, {
12+
"extensionFunction": {
13+
"functionAnchor": 1,
14+
"name": "lt:opt_i32_i32"
15+
}
16+
}
17+
],
18+
"relations": [{
19+
"root": {
20+
"input": {
21+
"project": {
22+
"common": {
23+
"direct": {}
24+
},
25+
"input": {
26+
"read": {
27+
"common": {
28+
"direct": {}
29+
},
30+
"baseSchema": {
31+
"names": ["key"],
32+
"struct": {
33+
"types": [{
34+
"i32": {
35+
"nullability": "NULLABILITY_NULLABLE"
36+
}
37+
}
38+
]
39+
},
40+
"partitionColumns": {
41+
"columnType": ["NORMAL_COL"]
42+
}
43+
},
44+
"filter": {
45+
"scalarFunction": {
46+
"functionReference": 2,
47+
"outputType": {
48+
"bool": {
49+
"nullability": "NULLABILITY_NULLABLE"
50+
}
51+
},
52+
"arguments": [{
53+
"value": {
54+
"scalarFunction": {
55+
"outputType": {
56+
"bool": {
57+
"nullability": "NULLABILITY_REQUIRED"
58+
}
59+
},
60+
"arguments": [{
61+
"value": {
62+
"selection": {
63+
"directReference": {
64+
"structField": {}
65+
}
66+
}
67+
}
68+
}
69+
]
70+
}
71+
}
72+
}, {
73+
"value": {
74+
"scalarFunction": {
75+
"functionReference": 1,
76+
"outputType": {
77+
"bool": {
78+
"nullability": "NULLABILITY_NULLABLE"
79+
}
80+
},
81+
"arguments": [{
82+
"value": {
83+
"selection": {
84+
"directReference": {
85+
"structField": {}
86+
}
87+
}
88+
}
89+
}, {
90+
"value": {
91+
"literal": {
92+
"i32": 3
93+
}
94+
}
95+
}
96+
]
97+
}
98+
}
99+
}
100+
]
101+
}
102+
},
103+
"localFiles": {
104+
"items": [{
105+
"uriFile": "file:///tmp/file.parquet",
106+
"length": "1486",
107+
"parquet": {}
108+
}
109+
]
110+
}
111+
}
112+
},
113+
"expressions": [{
114+
"cast": {
115+
"type": {
116+
"string": {
117+
"nullability": "NULLABILITY_NULLABLE"
118+
}
119+
},
120+
"input": {
121+
"selection": {
122+
"directReference": {
123+
"structField": {}
124+
}
125+
}
126+
},
127+
"failureBehavior": "FAILURE_BEHAVIOR_RETURN_NULL"
128+
}
129+
}
130+
]
131+
}
132+
},
133+
"names": ["key#173"]
134+
}
135+
}
136+
]
137+
}

velox/type/Filter.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -566,10 +566,10 @@ class BigintRange final : public Filter {
566566
: Filter(true, nullAllowed, FilterKind::kBigintRange),
567567
lower_(lowerExclusive ? lower + 1 : lower),
568568
upper_(upperExclusive ? upper - 1 : upper),
569-
lower32_(std::max<int64_t>(lower, std::numeric_limits<int32_t>::min())),
570-
upper32_(std::min<int64_t>(upper, std::numeric_limits<int32_t>::max())),
571-
lower16_(std::max<int64_t>(lower, std::numeric_limits<int16_t>::min())),
572-
upper16_(std::min<int64_t>(upper, std::numeric_limits<int16_t>::max())),
569+
lower32_(std::max<int64_t>(lower_, std::numeric_limits<int32_t>::min())),
570+
upper32_(std::min<int64_t>(upper_, std::numeric_limits<int32_t>::max())),
571+
lower16_(std::max<int64_t>(lower_, std::numeric_limits<int16_t>::min())),
572+
upper16_(std::min<int64_t>(upper_, std::numeric_limits<int16_t>::max())),
573573
isSingleValue_(upper_ == lower_) {}
574574

575575
std::unique_ptr<Filter> clone(

0 commit comments

Comments
 (0)