Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-62]Fixing issue0062 for package arrow dependencies in jar with refresh2 #172

Merged
merged 4 commits into from
Mar 22, 2021

Conversation

weiting-chen
Copy link
Collaborator

@weiting-chen weiting-chen commented Mar 18, 2021

What changes were proposed in this pull request?

Close #62
Close #66

The previous one will be drop, please use the new PR with refresh2

How was this patch tested?

The PR has been tested in my developing server.

@github-actions
Copy link

#62

@weiting-chen weiting-chen requested a review from zhouyuan March 18, 2021 15:47
@github-actions
Copy link


@weiting-chen
Copy link
Collaborator Author

It looks like the test has been passed, check if the formatting check can be ignored and any other concern under review, then I can merge the PR.

Copy link
Collaborator

@zhouyuan zhouyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like ARROW_ROOT is not correctly passed to cpp building
`mvn clean package -pl native-sql-engine/core -am -DskipTests -Dcpp_tests=ON -Dbuild_arrow=OFF -DSTATIC_ARROW=OFF -DBUILD_PROTOBUF=ON -DARROW_ROOT=/home/sparkuser/miniconda3/envs/pyarrow-dev/
....
[INFO] --- exec-maven-plugin:1.6.0:exec (Build cpp) @ spark-columnar-core ---
CMAKE Arguments:
TESTS=ON
BUILD_ARROW=OFF
STATIC_ARROW=OFF
BUILD_PROTOBUF=ON
ARROW_ROOT=/usr/local

`

@@ -13,6 +13,11 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)

set(CMAKE_BUILD_TYPE "Release")

set(ARROW_ROOT "/usr/local")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set(ARROW_ROOT "/usr/local")
set(ARROW_ROOT "/usr/local" CACHE PATH "Arrow root dir")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an upper/lower case issue, please try to use "lower cases in mvn" and "UPPER CASES in cpp".
it is a good finding, let me try if we can use the same cases in both mvn and cpp.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done to fix the issue for no default value in parameters.
The case sensitive usage for maven and cpp will be kept since coding style concern:
mvn -> lowercase, cpp -> UPPERCASE

@github-actions
Copy link


@github-actions
Copy link


@zhouyuan
Copy link
Collaborator

@weiting-chen the new building process looks good for me. one question: as native sql engine depends arrow data source, we'll need to apply the same building process for arrow-data-source module first, is it convenient to apply the same method for arrow data source?

@weiting-chen
Copy link
Collaborator Author

@weiting-chen the new building process looks good for me. one question: as native sql engine depends arrow data source, we'll need to apply the same building process for arrow-data-source module first, is it convenient to apply the same method for arrow data source?

Per discussion, lets merge the PR first and backport it to v1.1 branch since this PR also have to pass Gold Deck testing.
And I will file another issue to address the feature you mentioned.

@weiting-chen weiting-chen merged commit 13c66f4 into oap-project:master Mar 22, 2021
zhouyuan added a commit that referenced this pull request Mar 29, 2021
* [NSE-130] support decimal round and abs (#166)

* support decimal round and abs

* remove duplicate cast in multiply/divide

* [NSE-161] adding format check (#165)

* adding format check

Signed-off-by: Yuan Zhou <[email protected]>

* formating code

Signed-off-by: Yuan Zhou <[email protected]>

* adding google format

Signed-off-by: Yuan Zhou <[email protected]>

* reformat with new style

Signed-off-by: Yuan Zhou <[email protected]>

* lower clang version to 10

Signed-off-by: Yuan Zhou <[email protected]>

* adding script to format code

Signed-off-by: Yuan Zhou <[email protected]>

* [NSE-170]improve sort shuffle code (#171)

* improve sort shuffle code

Signed-off-by: Yuan Zhou <[email protected]>

* fix format

Signed-off-by: Yuan Zhou <[email protected]>

* pass by ref in builder

Signed-off-by: Yuan Zhou <[email protected]>

* fix string/decimal builder

Signed-off-by: Yuan Zhou <[email protected]>

* [NSE-62]Fixing issue0062 for package arrow dependencies in jar with refresh2 (#172)

* Add arrow build and dependency support

* Add compress.sh default value

* Fix bug for parameter's default value

* Add CACHE PATH

* fix copy bitmap in InplaceSort (#174)

* [NSE-153] fix window results (#175)

* fix window sort memory

Signed-off-by: Yuan Zhou <[email protected]>

* remove unused code

Signed-off-by: Yuan Zhou <[email protected]>

* fix windown w/o avg

Signed-off-by: Yuan Zhou <[email protected]>

* fix format

Signed-off-by: Yuan Zhou <[email protected]>

* fix decimal sort

Signed-off-by: Yuan Zhou <[email protected]>

* Fix issue 179 for arrow include directory (#181)

* Fix issue0191 for .so file copy to tmp. (#192)

* Following NSE-153, optimize fallback conditions for columnar window (#189)

* Fix q14a/b segfault (#193)

Signed-off-by: Chendi Xue <[email protected]>

* [NSE-194]Turn on several Arrow parameters (#195)

* Turn on several Arrow parameters

* Change SIMD Level Setting

* Hashmap build opt for semi/anti/exists join (#197)

Signed-off-by: Chendi Xue <[email protected]>

* [NSE-198] support the month() and dayofmonth() functions with DateType (#199)

* [NSE-206] fix doc link, update limitations (#205)

* fix doc link

Signed-off-by: Yuan Zhou <[email protected]>

* update arrow data source

Signed-off-by: Yuan Zhou <[email protected]>

* adding limitations

Signed-off-by: Yuan Zhou <[email protected]>

* mention limits

Signed-off-by: Yuan Zhou <[email protected]>

* [NSE-170] using unsafe appender (#203)

This patch adds an unsafe appender which will reserve space before builder array.
The boolean builder is not touched as only malloc small sized memory
The string builder are not touched as it's diffcult to pre-allocate the space

* using unsafe appender

Signed-off-by: Yuan Zhou <[email protected]>

* fix format

Signed-off-by: Yuan Zhou <[email protected]>

* update arrow branch

Signed-off-by: Yuan Zhou <[email protected]>

Co-authored-by: Rui Mo <[email protected]>
Co-authored-by: Wei-Ting Chen <[email protected]>
Co-authored-by: Hongze Zhang <[email protected]>
Co-authored-by: Chendi.Xue <[email protected]>
Co-authored-by: JiaKe <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants