Bikramjeet Vig has posted comments on this change. Change subject: IMPALA-4795: Allow fetching function obj from catalog using signature ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/7479/1/fe/src/main/java/org/apache/impala/catalog/Catalog.java File fe/src/main/java/org/apache/impala/catalog/Catalog.java: PS1, Line 463: TFunction tfn = objectDesc.getFn(); : Function fn = null; : if (tfn.isSetScalar_fn() || tfn.isSetAggregate_fn()) { : Function desc = Function.fromThrift(tfn); : fn = getFunction(desc, Function.CompareMode.IS_INDISTINGUISHABLE); : } else if (tfn.isSetSignature()) { : Db db = getDb(tfn.getName().getDb_name()); : fn = (db == null) ? null : db.getFunction(tfn.getSignature()); : } > Pulling into a separate fn with a clear interface in the javadoc makes sens Done PS1, Line 463: TFunction tfn = objectDesc.getFn(); : Function fn = null; : if (tfn.isSetScalar_fn() || tfn.isSetAggregate_fn()) { : Function desc = Function.fromThrift(tfn); : fn = getFunction(desc, Function.CompareMode.IS_INDISTINGUISHABLE); : } else if (tfn.isSetSignature()) { : Db db = getDb(tfn.getName().getDb_name()); : fn = (db == null) ? null : db.getFunction(tfn.getSignature()); : } > nit: How about moving this to a new method, getFunction(TFunction fn, mode) Done http://gerrit.cloudera.org:8080/#/c/7479/1/fe/src/main/java/org/apache/impala/catalog/Function.java File fe/src/main/java/org/apache/impala/catalog/Function.java: Line 360: throw new IllegalStateException("Expected function type to be either UDA or UDF."); > I believe IMPALA-626 is what caused the problem filed in IMPALA-4795 in the Done http://gerrit.cloudera.org:8080/#/c/7479/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: PS1, Line 148: input_url = self.CATALOG_OBJECT_URL.format("25000") + \ : "?object_type=FUNCTION&object_name=_impala_builtins.abs(BIGINT)" : response = requests.get(input_url) : assert response.status_code == requests.codes.ok \ : and > nit: I think you could use impalad.get_catalog_object_dump(..) Done Also found and fixed a bug in get_catalog_object_dump -- To view, visit http://gerrit.cloudera.org:8080/7479 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2cfad0213a79d39b77ad9aff701a93f93be4bf7f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
