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 <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoram Thanga <[email protected]> Gerrit-Comment-Date: Wed, 13 Feb 2019 17:39:58 +0000 Gerrit-HasComments: Yes
