Skip to content

Commit 23222a5

Browse files
mshang816mbasmanovamajetideepak
committed
[Native] Update CI Image and advance Velox
Add new CI image Add -DFOLLY_HAVE_INT128_T=ON to centos setup script Add known warnings to Linux Advance Velox Add support for aggregations over sorted inputs Co-authored-by: Masha Basmanova <mbasmanova@fb.com> Co-authored-by: Deepak Majeti <deepak.majeti@ibm.com>
1 parent 07b9712 commit 23222a5

8 files changed

Lines changed: 33 additions & 40 deletions

File tree

‎.circleci/continue_config.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ workflows:
3232
executors:
3333
build:
3434
docker:
35-
- image: prestocpp/prestocpp-avx-centos:kpai-20221018
35+
- image: prestocpp/prestocpp-avx-centos:root-20230613
3636
resource_class: 2xlarge
3737
environment:
3838
MAVEN_OPTS: "-Xmx4G -XX:+ExitOnOutOfMemoryError"

‎.github/workflows/test-native-specific.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ jobs:
103103

104104
native-specific-linux:
105105
runs-on: ubuntu-latest
106-
container: prestocpp/prestocpp-avx-centos:kpai-20221018
106+
container: prestocpp/prestocpp-avx-centos:root-20230613
107107

108108
env:
109109
CC: /opt/rh/gcc-toolset-9/root/bin/gcc

‎.github/workflows/test-native.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ env:
2323
jobs:
2424
build-and-test:
2525
runs-on: ubuntu-latest
26-
container: prestocpp/prestocpp-avx-centos:kpai-20221018
26+
container: prestocpp/prestocpp-avx-centos:root-20230613
2727

2828
env:
2929
CC: /opt/rh/gcc-toolset-9/root/bin/gcc

‎presto-native-execution/CMakeLists.txt‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,8 @@ endif()
3333

3434
set(KNOWN_WARNINGS "-Wno-nullability-completeness")
3535

36-
# known warnings for MacOS
37-
if(${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
38-
set(KNOWN_WARNINGS "${KNOWN_WARNINGS} -Wno-deprecated-declarations")
39-
endif()
36+
# known warnings
37+
set(KNOWN_WARNINGS "${KNOWN_WARNINGS} -Wno-deprecated-declarations")
4038

4139
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${KNOWN_WARNINGS}")
4240

‎presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1455,26 +1455,30 @@ VeloxQueryPlanConverterBase::toVeloxQueryPlan(
14551455
const std::shared_ptr<protocol::TableWriteInfo>& tableWriteInfo,
14561456
const protocol::TaskId& taskId) {
14571457
std::vector<std::string> aggregateNames;
1458-
std::vector<core::CallTypedExprPtr> aggregates;
1458+
std::vector<core::AggregationNode::Aggregate> aggregates;
14591459
std::vector<core::FieldAccessTypedExprPtr> aggrMasks;
14601460
aggregateNames.reserve(node->aggregations.size());
14611461
aggregates.reserve(node->aggregations.size());
1462-
aggrMasks.reserve(node->aggregations.size());
1463-
for (const auto& [variable, aggregation] : node->aggregations) {
1464-
VELOX_USER_CHECK_NULL(
1465-
aggregation.orderBy,
1466-
"Aggregations with ORDER BY are not supported yet.");
1467-
VELOX_USER_CHECK_NULL(
1468-
aggregation.filter, "Aggregations with filters are not supported");
1469-
aggregateNames.emplace_back(variable.name);
1470-
aggregates.emplace_back(
1471-
std::dynamic_pointer_cast<const core::CallTypedExpr>(
1472-
exprConverter_.toVeloxExpr(aggregation.call)));
1473-
if (aggregation.mask == nullptr) {
1474-
aggrMasks.emplace_back(nullptr);
1475-
} else {
1476-
aggrMasks.emplace_back(exprConverter_.toVeloxExpr(aggregation.mask));
1462+
for (const auto& entry : node->aggregations) {
1463+
aggregateNames.emplace_back(entry.first.name);
1464+
1465+
core::AggregationNode::Aggregate aggregate;
1466+
aggregate.call = std::dynamic_pointer_cast<const core::CallTypedExpr>(
1467+
exprConverter_.toVeloxExpr(entry.second.call));
1468+
if (entry.second.mask != nullptr) {
1469+
aggregate.mask = exprConverter_.toVeloxExpr(entry.second.mask);
1470+
}
1471+
1472+
if (entry.second.orderBy) {
1473+
for (const auto& orderBy : entry.second.orderBy->orderBy) {
1474+
aggregate.sortingKeys.emplace_back(
1475+
exprConverter_.toVeloxExpr(orderBy.variable));
1476+
aggregate.sortingOrders.emplace_back(
1477+
toVeloxSortOrder(orderBy.sortOrder));
1478+
}
14771479
}
1480+
1481+
aggregates.emplace_back(aggregate);
14781482
}
14791483

14801484
core::AggregationNode::Step step;
@@ -1507,7 +1511,6 @@ VeloxQueryPlanConverterBase::toVeloxQueryPlan(
15071511
: std::vector<core::FieldAccessTypedExprPtr>{},
15081512
aggregateNames,
15091513
aggregates,
1510-
aggrMasks,
15111514
false, // ignoreNullKeys
15121515
toVeloxQueryPlan(node->source, tableWriteInfo, taskId));
15131516
}
@@ -1584,8 +1587,7 @@ VeloxQueryPlanConverterBase::toVeloxQueryPlan(
15841587
toVeloxExprs(node->distinctVariables),
15851588
std::vector<core::FieldAccessTypedExprPtr>{},
15861589
std::vector<std::string>{}, // aggregateNames
1587-
std::vector<core::CallTypedExprPtr>{}, // aggregates
1588-
std::vector<core::FieldAccessTypedExprPtr>{}, // aggrMasks
1590+
std::vector<core::AggregationNode::Aggregate>{}, // aggregates
15891591
false, // ignoreNullKeys
15901592
toVeloxQueryPlan(node->source, tableWriteInfo, taskId)));
15911593
}

‎presto-native-execution/scripts/setup-centos.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export COMPILER_FLAGS=$(echo -n $(get_cxx_flags $CPU_TARGET))
6767
git clone https://github.com/facebook/folly &&
6868
cd folly &&
6969
git checkout $FB_OS_VERSION &&
70-
cmake_install -DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=ON
70+
cmake_install -DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=ON -DFOLLY_HAVE_INT128_T=ON
7171
)
7272

7373
(

‎presto-native-execution/src/test/java/com/facebook/presto/nativeworker/AbstractTestNativeAggregations.java‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,13 @@ public void testAggregations()
6262

6363
assertQuery("SELECT sum(custkey), clerk FROM orders GROUP BY clerk HAVING sum(custkey) > 10000");
6464

65-
// TODO results from array_agg() are not deterministic so we just compare cardinality for the time being
66-
// We can switch to array_sort() once it becomes available from velox
67-
assertQuery("SELECT orderkey, cardinality(array_agg(linenumber)) FROM lineitem GROUP BY 1");
65+
assertQuery("SELECT orderkey, array_sort(array_agg(linenumber)) FROM lineitem GROUP BY 1");
6866
assertQuery("SELECT orderkey, map_agg(linenumber, discount) FROM lineitem GROUP BY 1");
6967

70-
// TODO results from map_union() are not deterministic so we just compare cardinality for the time being
71-
assertQuery("SELECT cardinality(map_union(quantity_by_linenumber)) FROM orders_ex");
68+
assertQuery("SELECT array_agg(nationkey ORDER BY name) FROM nation");
69+
assertQuery("SELECT orderkey, array_agg(quantity ORDER BY linenumber DESC) FROM lineitem GROUP BY 1");
70+
71+
assertQuery("SELECT map_keys(map_union(quantity_by_linenumber)) FROM orders_ex");
7272

7373
assertQuery("SELECT orderkey, count_if(linenumber % 2 > 0) FROM lineitem GROUP BY 1");
7474
assertQuery("SELECT orderkey, bool_and(linenumber % 2 = 1) FROM lineitem GROUP BY 1");
@@ -286,13 +286,6 @@ public void testMarkDistinct()
286286
assertQuery("SELECT orderkey, count(distinct comment), sum(distinct linenumber) FROM lineitem GROUP BY 1");
287287
}
288288

289-
@Test
290-
public void testUnsupported()
291-
{
292-
assertQueryFails("SELECT array_agg(nationkey ORDER BY name) FROM nation",
293-
".* Aggregations with ORDER BY are not supported yet.");
294-
}
295-
296289
private void assertQueryResultCount(String sql, int expectedResultCount)
297290
{
298291
assertEquals(getQueryRunner().execute(sql).getRowCount(), expectedResultCount);

‎presto-native-execution/velox‎

Submodule velox updated 175 files

0 commit comments

Comments
 (0)