-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[New Featrue] Support Vectorization Execution Engine Interface For Doris #6329
Conversation
What does point 3 and point 4 specifically refer to? |
be/CMakeLists.txt
Outdated
@@ -704,3 +704,8 @@ install(FILES | |||
${BASE_DIR}/../conf/odbcinst.ini | |||
DESTINATION ${OUTPUT_DIR}/conf) | |||
|
|||
|
|||
get_property(dirs DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be we should remove cmake log ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I will remove it
@@ -1925,8 +1939,7 @@ DoubleVal AggregateFunctions::knuth_var_pop_finalize(FunctionContext* ctx, | |||
} | |||
|
|||
DecimalV2Val AggregateFunctions::decimalv2_knuth_var_pop_finalize(FunctionContext* ctx, | |||
const StringVal& state_sv) { | |||
DCHECK(!state_sv.is_null); | |||
const StringVal& state_sv) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a code format here?
@@ -84,7 +92,7 @@ fi | |||
|
|||
eval set -- "$OPTS" | |||
|
|||
PARALLEL=$[$(nproc)/4+1] | |||
PARALLEL=$[$(nproc)+1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set PARALLEL=
@@ -17,6 +17,9 @@ | |||
|
|||
package org.apache.doris.analysis; | |||
|
|||
import java.util.Arrays; | |||
import java.util.Map; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import order
@@ -17,6 +17,9 @@ | |||
|
|||
package org.apache.doris.analysis; | |||
|
|||
import com.google.common.collect.Lists; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import order
@@ -20,6 +20,7 @@ | |||
import static org.apache.doris.common.io.IOUtils.readOptionStringOrNull; | |||
import static org.apache.doris.common.io.IOUtils.writeOptionString; | |||
|
|||
import avro.shaded.com.google.common.collect.ImmutableSet; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong import
@@ -1020,12 +1121,21 @@ private void initAggregateBuiltins() { | |||
// count(*) | |||
addBuiltin(AggregateFunction.createBuiltin(FunctionSet.COUNT, | |||
new ArrayList<Type>(), Type.BIGINT, Type.BIGINT, | |||
prefix + "9init_zeroIN9doris_udf9BigIntValEEEvPNS2_15FunctionContextEPT_", | |||
prefix + "18init_zero_not_nullIN9doris_udf9BigIntValEEEvPNS2_15FunctionContextEPT_", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, to keep count slot not null.
@@ -1037,12 +1147,21 @@ private void initAggregateBuiltins() { | |||
// Count | |||
addBuiltin(AggregateFunction.createBuiltin(FunctionSet.COUNT, | |||
Lists.newArrayList(t), Type.BIGINT, Type.BIGINT, | |||
prefix + "9init_zeroIN9doris_udf9BigIntValEEEvPNS2_15FunctionContextEPT_", | |||
prefix + "18init_zero_not_nullIN9doris_udf9BigIntValEEEvPNS2_15FunctionContextEPT_", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed?
@@ -1479,7 +1845,7 @@ private void initAggregateBuiltins() { | |||
prefix + "13rank_finalizeEPN9doris_udf15FunctionContextERNS1_9StringValE")); | |||
addBuiltin(AggregateFunction.createAnalyticBuiltin( "row_number", | |||
new ArrayList<Type>(), Type.BIGINT, Type.BIGINT, | |||
prefix + "9init_zeroIN9doris_udf9BigIntValEEEvPNS2_15FunctionContextEPT_", | |||
prefix + "18init_zero_not_nullIN9doris_udf9BigIntValEEEvPNS2_15FunctionContextEPT_", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed?
@@ -216,6 +216,8 @@ | |||
public static final int VERSION_101 = 101; | |||
// add data encrypt | |||
public static final int VERSION_102 = 102; | |||
// add vectorized to function | |||
public static final int VERSION_103 = 103; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used?
@@ -104,40 +117,49 @@ | |||
private HdfsURI location; | |||
private TFunctionBinaryType binaryType; | |||
|
|||
protected NullableMode nullableMode = NullableMode.DEPEND_ON_ARGUMENT; | |||
|
|||
private boolean vectorized; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private boolean vectorized; | |
private boolean vectorized = false; |
@@ -640,6 +664,9 @@ public void readFields(DataInput input) throws IOException { | |||
if (hasChecksum) { | |||
checksum = Text.readString(input); | |||
} | |||
if (Catalog.getCurrentCatalogJournalVersion() >= FeMetaVersion.VERSION_101) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove meta version
a037693
to
250ad89
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@HappenLee does those 2 PR all of implementation of ver-engine? |
Not yet, more PR will coming. |
@francisoliverlee yes,but it only contain the some interface of vec-engine. |
Great, I look forward to it |
that's nice, waiting...o_o |
…ris (apache#6329) 1. FE vectorized plan code 2. Function register vec function 3. Diff function nullable type 4. New thirdparty code and new thrift struct
…ris (apache#6329) 1. FE vectorized plan code 2. Function register vec function 3. Diff function nullable type 4. New thirdparty code and new thrift struct
…he#6341 and add back in apache#6329 by mistake
…he#6341 and add back in apache#6329 by mistake
…he#6341 and add back in apache#6329 by mistake (apache#9233)
…he#6341 and add back in apache#6329 by mistake (apache#9233)
…he#6341 and add back in apache#6329 by mistake (apache#9233)
Proposed changes
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.
Types of changes
What types of changes does your code introduce to Doris?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.