Skip to content

DRILL-7343: Add User-Agent UDFs to Drill#1840

Closed
cgivre wants to merge 1 commit intoapache:masterfrom
cgivre:udf_useragent
Closed

DRILL-7343: Add User-Agent UDFs to Drill#1840
cgivre wants to merge 1 commit intoapache:masterfrom
cgivre:udf_useragent

Conversation

@cgivre
Copy link
Contributor

@cgivre cgivre commented Aug 12, 2019

These UDFs add the ability to parse user agent strings, which is useful for security data analysis.

@cgivre cgivre added the enhancement PRs that add a new functionality to Drill label Aug 12, 2019
@cgivre
Copy link
Contributor Author

cgivre commented Sep 4, 2019

@KazydubB
What happens in the parse_query function is that there is a duplicate function of the same name which allows null input which returns an empty list. I had tried this before, but apparently didn't use the correct parameters so it didn't work for me, BUT, now it does so yay!

In any event, this seems like horrible design. I'm going to open a JIRA to create a new function handler which returns an empty list on null. That seems like a better approach than having to write 2 UDFS for every UDF with a complex writer output.

I also added a series of unit tests to test this functionality.

@KazydubB
Copy link
Member

KazydubB commented Sep 4, 2019

@cgivre I agree that having two functions for OPTIONAL and REQUIRED cases is not the best experience, but returning an empty list on NULL is not enough, because there may be functions that will need to return a MAP for example. So, there's a need to provide NullHandling for NULL input for other cases too (EMPTY_LIST_IF_NULL, EMPTY_MAP_IF_NULL etc.).

@cgivre
Copy link
Contributor Author

cgivre commented Sep 4, 2019

@KazydubB I opened a JIRA for the null issues, but this PR should be ready to go.

Copy link
Member

@arina-ielchiieva arina-ielchiieva left a comment

Choose a reason for hiding this comment

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

@cgivre overall looks good, please fix minor remaining issues (including extra or missing spaces) and PR will be ready to go.

@cgivre cgivre force-pushed the udf_useragent branch 2 times, most recently from 0605d37 to 81c9945 Compare September 5, 2019 13:08
@arina-ielchiieva
Copy link
Member

Looks good, +1

@gparai gparai closed this in bd6d7b1 Sep 8, 2019
xiangt920 pushed a commit to xiangt920/drill that referenced this pull request Dec 26, 2019
@cgivre cgivre deleted the udf_useragent branch January 16, 2020 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement PRs that add a new functionality to Drill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants