You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2019/02/12 16:14:22 UTC

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default 

Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12451


Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................

IMPALA-8187: UDF samples hide symbols by default

Testing:
Ran UDF tests, confirmed that changing the compile flags without adding
IMPALA_UDF_EXPORT caused them to fail because they couldn't find
the symbols.

Added a test to check that symbols are actually hidden.

Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
---
M be/src/testutil/CMakeLists.txt
M be/src/testutil/test-udas.cc
M be/src/testutil/test-udfs.cc
M be/src/udf/udf.h
M be/src/udf_samples/CMakeLists.txt
M be/src/udf_samples/hyperloglog-uda.cc
M be/src/udf_samples/uda-sample.cc
M be/src/udf_samples/udf-sample.cc
M tests/query_test/test_udfs.py
9 files changed, 168 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/12451/3
-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Zoram Thanga (Code Review)" <ge...@cloudera.org>.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

Thanks for doing this! LGTM.

http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py@440
PS3, Line 440:         create function `{0}`.unexported() returns BIGINT LOCATION '{1}'
Should this be unexported_symbol()?



-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 00:33:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 18:12:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3769/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 18:12:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................

IMPALA-8187: UDF samples hide symbols by default

Testing:
Ran UDF tests, confirmed that changing the compile flags without adding
IMPALA_UDF_EXPORT caused them to fail because they couldn't find
the symbols.

Added a test to check that symbols are actually hidden.

Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Reviewed-on: http://gerrit.cloudera.org:8080/12451
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/testutil/CMakeLists.txt
M be/src/testutil/test-udas.cc
M be/src/testutil/test-udfs.cc
M be/src/udf/udf.h
M be/src/udf_samples/CMakeLists.txt
M be/src/udf_samples/hyperloglog-uda.cc
M be/src/udf_samples/uda-sample.cc
M be/src/udf_samples/udf-sample.cc
M tests/query_test/test_udfs.py
10 files changed, 188 insertions(+), 3 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 6: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 22:19:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Zoram Thanga, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12451

to look at the new patch set (#5).

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................

IMPALA-8187: UDF samples hide symbols by default

Testing:
Ran UDF tests, confirmed that changing the compile flags without adding
IMPALA_UDF_EXPORT caused them to fail because they couldn't find
the symbols.

Added a test to check that symbols are actually hidden.

Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
---
M be/CMakeLists.txt
M be/src/testutil/CMakeLists.txt
M be/src/testutil/test-udas.cc
M be/src/testutil/test-udfs.cc
M be/src/udf/udf.h
M be/src/udf_samples/CMakeLists.txt
M be/src/udf_samples/hyperloglog-uda.cc
M be/src/udf_samples/uda-sample.cc
M be/src/udf_samples/udf-sample.cc
M tests/query_test/test_udfs.py
10 files changed, 188 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/12451/5
-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/udf_samples/CMakeLists.txt
File be/src/udf_samples/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/udf_samples/CMakeLists.txt@27
PS3, Line 27: set(IR_COMPILE_FLAGS "-emit-llvm" "-O3" "-std=c++14" "-c" "-I../" ${CLANG_BASE_FLAGS})
> Do we care about this case as well ?
Actually, since it's emitting IR, it's irrelevant.



-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 00:23:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/testutil/test-udas.cc
File be/src/testutil/test-udas.cc:

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/testutil/test-udas.cc@343
PS3, Line 343: }
nit: Please keep a blank line between functions


http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/udf_samples/CMakeLists.txt
File be/src/udf_samples/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/udf_samples/CMakeLists.txt@27
PS3, Line 27: set(IR_COMPILE_FLAGS "-emit-llvm" "-O3" "-std=c++14" "-c" "-I../" ${CLANG_BASE_FLAGS})
Do we care about this case as well ?



-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 00:10:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 18:03:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12451/4/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/12451/4/tests/query_test/test_udfs.py@442
PS4, Line 442: u
flake8: E126 continuation line over-indented for hanging indent


http://gerrit.cloudera.org:8080/#/c/12451/4/tests/query_test/test_udfs.py@449
PS4, Line 449: u
flake8: E126 continuation line over-indented for hanging indent



-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 17:37:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3:

Zoram, can you please also take a look ?


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 00:10:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/udf_samples/CMakeLists.txt
File be/src/udf_samples/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/12451/3/be/src/udf_samples/CMakeLists.txt@27
PS3, Line 27: # Function to generate rule to cross compile a source file to an IR module.
> Actually, since it's emitting IR, it's irrelevant.
I think this was a good question. I made the changes to the CMakeLists to pass the same flags to the IR (refactoring a little in the process). I think it is actually a good practice for IR to avoid weak symbols being merged as part of linking.

I confirmed that symbol visibility is actually ignored: IMPALA-8196

That part might be a little academic since everything is exposed in the IR UDFs anyway, but it's worth testing the behaviour to make sure it doesn't blow up.


http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py@440
PS3, Line 440:         create function `{0}`.unexported() returns BIGINT LOCATION '{1}'
> Should this be unexported_symbol()?
The name of the UDF doesn't need to match the symbol for C++ udfs.


http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py@442
PS3, Line 442: u
> flake8: E126 continuation line over-indented for hanging indent
Done


http://gerrit.cloudera.org:8080/#/c/12451/4/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/12451/4/tests/query_test/test_udfs.py@442
PS4, Line 442: u
> flake8: E126 continuation line over-indented for hanging indent
Done


http://gerrit.cloudera.org:8080/#/c/12451/4/tests/query_test/test_udfs.py@449
PS4, Line 449: u
> flake8: E126 continuation line over-indented for hanging indent
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 17:39:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2098/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 18:12:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2085/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 17:00:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Zoram Thanga, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12451

to look at the new patch set (#4).

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................

IMPALA-8187: UDF samples hide symbols by default

Testing:
Ran UDF tests, confirmed that changing the compile flags without adding
IMPALA_UDF_EXPORT caused them to fail because they couldn't find
the symbols.

Added a test to check that symbols are actually hidden.

Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
---
M be/CMakeLists.txt
M be/src/testutil/CMakeLists.txt
M be/src/testutil/test-udas.cc
M be/src/testutil/test-udfs.cc
M be/src/udf/udf.h
M be/src/udf_samples/CMakeLists.txt
M be/src/udf_samples/hyperloglog-uda.cc
M be/src/udf_samples/uda-sample.cc
M be/src/udf_samples/udf-sample.cc
M tests/query_test/test_udfs.py
10 files changed, 188 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/51/12451/4
-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2099/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 18:23:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/12451/3/tests/query_test/test_udfs.py@442
PS3, Line 442: u
flake8: E126 continuation line over-indented for hanging indent



-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Feb 2019 16:15:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8187: UDF samples hide symbols by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12451 )

Change subject: IMPALA-8187: UDF samples hide symbols by default
......................................................................


Patch Set 3: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/12451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie17b74b9bce9cc6962393017879b65409ce23b28
Gerrit-Change-Number: 12451
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 00:10:18 +0000
Gerrit-HasComments: No