You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hawq.apache.org by zhangjackey <gi...@git.apache.org> on 2018/05/08 03:39:21 UTC
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
GitHub user zhangjackey opened a pull request:
https://github.com/apache/incubator-hawq/pull/1361
HAWQ-1612. Implement vectorized aggregate functions, such as sum/count/avg.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zhangjackey/incubator-hawq v_agg
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-hawq/pull/1361.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1361
----
commit 193920c800d5d06abc341a6dc9dc4df14ed4aa30
Author: Shujie Zhang <sh...@...>
Date: 2018-05-08T03:10:04Z
implement some vectorized aggregate functions, such as sum/count/avg.
commit 32d5480ec25f5054e2d8d2c820ecda40d4cd9a87
Author: Shujie Zhang <sh...@...>
Date: 2018-05-08T03:17:07Z
implement vectorized aggregate functions, this commit is the test case for it.
----
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by zhangjackey <gi...@git.apache.org>.
Github user zhangjackey closed the pull request at:
https://github.com/apache/incubator-hawq/pull/1361
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187231898
--- Diff: contrib/vexecutor/vagg.h ---
@@ -0,0 +1,94 @@
+#ifndef VAGG_H
+#define VAGG_H
+
+#include "executor/execHHashagg.h"
+
+/* batch hashagg group linklist header */
+typedef struct GroupData {
+ HashAggEntry *entry; // pointer to agg_hash_entry
+ int idx; // pointer to idx_list
+} GroupData;
+
+/* batch hashagg group data */
+typedef struct BatchAggGroupData{
+ GroupData group_header[1024]; //group linklist header
--- End diff --
1024, use a macro is better.
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187230968
--- Diff: contrib/vexecutor/vagg.h ---
@@ -0,0 +1,94 @@
+#ifndef VAGG_H
+#define VAGG_H
+
+#include "executor/execHHashagg.h"
+
+/* batch hashagg group linklist header */
+typedef struct GroupData {
+ HashAggEntry *entry; // pointer to agg_hash_entry
+ int idx; // pointer to idx_list
--- End diff --
Refine my poor comments:
1. use /**/ instead //.
2. unify the ident.
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by zhangjackey <gi...@git.apache.org>.
Github user zhangjackey commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187854511
--- Diff: contrib/vexecutor/vagg.h ---
@@ -0,0 +1,94 @@
+#ifndef VAGG_H
+#define VAGG_H
+
+#include "executor/execHHashagg.h"
+
+/* batch hashagg group linklist header */
+typedef struct GroupData {
+ HashAggEntry *entry; // pointer to agg_hash_entry
+ int idx; // pointer to idx_list
+} GroupData;
+
+/* batch hashagg group data */
+typedef struct BatchAggGroupData{
+ GroupData group_header[1024]; //group linklist header
--- End diff --
BATCHSIZE is perfect， I will change to it in next commit.
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by weinan003 <gi...@git.apache.org>.
Github user weinan003 commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187261983
--- Diff: contrib/vexecutor/vagg.h ---
@@ -0,0 +1,94 @@
+#ifndef VAGG_H
+#define VAGG_H
+
+#include "executor/execHHashagg.h"
+
+/* batch hashagg group linklist header */
+typedef struct GroupData {
+ HashAggEntry *entry; // pointer to agg_hash_entry
+ int idx; // pointer to idx_list
+} GroupData;
+
+/* batch hashagg group data */
+typedef struct BatchAggGroupData{
+ GroupData group_header[1024]; //group linklist header
--- End diff --
variable **BATCHSIZE** in *vexecutor.c* indicate the number of slots the TupleBatch hold.
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by zhangjackey <gi...@git.apache.org>.
Github user zhangjackey commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187854332
--- Diff: contrib/vexecutor/vexecutor.c ---
@@ -117,7 +122,7 @@ static void backportTupleDescriptor(PlanState* ps,TupleDesc td)
tuple = caql_getnext(pcqCtx);
if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for type %u", oidtypeid);
+ continue;
--- End diff --
For the grouping aggregation, there may generate the tuple descriptor like this: {vtype, vtype, non-vtype, non-vtype}, we have to convert the vtype to non-vtype and skip the non-vtype which already exists.
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by zhangjackey <gi...@git.apache.org>.
Github user zhangjackey commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187854688
--- Diff: contrib/vexecutor/vagg.h ---
@@ -0,0 +1,94 @@
+#ifndef VAGG_H
+#define VAGG_H
+
+#include "executor/execHHashagg.h"
+
+/* batch hashagg group linklist header */
+typedef struct GroupData {
+ HashAggEntry *entry; // pointer to agg_hash_entry
+ int idx; // pointer to idx_list
--- End diff --
I will change it in next commit : )
---
[GitHub] incubator-hawq pull request #1361: HAWQ-1612. Implement vectorized aggregate...
Posted by interma <gi...@git.apache.org>.
Github user interma commented on a diff in the pull request:
https://github.com/apache/incubator-hawq/pull/1361#discussion_r187231278
--- Diff: contrib/vexecutor/vexecutor.c ---
@@ -117,7 +122,7 @@ static void backportTupleDescriptor(PlanState* ps,TupleDesc td)
tuple = caql_getnext(pcqCtx);
if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for type %u", oidtypeid);
+ continue;
--- End diff --
Why use `continue` instead of `elog ERROR` here?
---