Skip to content

Conversation

@sandeep-katta
Copy link
Contributor

@sandeep-katta sandeep-katta commented Aug 22, 2019

What changes were proposed in this pull request?

Add DROP FUNCTION sql description in SQL reference

Why are the changes needed?

Currently from spark there is no complete sql guide is present, so it is better to document all the sql commands, this jira is sub part of this task.

Does this PR introduce any user-facing change?

Yes before user cannot find any reference for drop function command in the spark docs.

After Fix:
image

image

image

How was this patch tested?

tested with jekyll build

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not sound right. We are talking about create in a drop action ?

Copy link
Contributor

Choose a reason for hiding this comment

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

How about -
"If specified, no exception is thrown when the function does not exist"

@dilipbiswal
Copy link
Contributor

@sandeep-katta Hello, could you please update this by following the convention of existing merged PRS. Thanks a lot.
#25529
#25525

@sandeep-katta
Copy link
Contributor Author

@dilipbiswal can you help me review this please


**This page is under construction**
### Description
Dropping a user-defined function(UDF). An exception will be thrown if the function does not exist
Copy link
Contributor

Choose a reason for hiding this comment

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

DROP FUNCTION statement drops a temporary or user defined function(UDF)

### Example
{% highlight sql %}
-- Create a permanent function `test_avg`

Copy link
Contributor

Choose a reason for hiding this comment

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

delete this line ?

CREATE FUNCTION test_avg as 'org.apache.hadoop.hive.ql.udf.generic.GenericUDAFAverage';

-- List user functions

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: delete this line ?

-- List user functions

SHOW user functions;

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: delete this line ?


SHOW user functions;

+-------------------+
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Shift this by 2 space to align with other prs.

-- Drop Permanent function

DROP FUNCTION test_avg;
+---------+
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Shift this by 2 space to align with other prs.

CREATE TEMPORARY FUNCTION temp_avg as 'org.apache.hadoop.hive.ql.udf.generic.GenericUDAFAverage';

-- Drop Temporary function

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: delete this line.

-- Drop Temporary function

DROP TEMPORARY FUNCTION IF EXISTS temp_avg;
+---------+
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Shift this by 2 space to align with other prs.

| Result |
+---------+
+---------+

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: delete this line ?

@dilipbiswal
Copy link
Contributor

LGTM
cc @gatorsmile

@gatorsmile
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Sep 15, 2019

Test build #110628 has finished for PR 25553 at commit f458a7e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

CREATE FUNCTION test_avg as 'org.apache.hadoop.hive.ql.udf.generic.GenericUDAFAverage';

-- List user functions
SHOW user functions;
Copy link
Member

Choose a reason for hiding this comment

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

SHOW user functions; -> SHOW USER FUNCTIONS;?

@SparkQA
Copy link

SparkQA commented Sep 17, 2019

Test build #110734 has finished for PR 25553 at commit 9b3e2f9.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@gatorsmile
Copy link
Member

We need to show the precedence of temp view and table when they share the same name.

@SparkQA
Copy link

SparkQA commented Sep 17, 2019

Test build #110770 has finished for PR 25553 at commit 54aad6b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

| function |
+-------------------+
| default.test_avg |
| temp_avg |
Copy link
Member

Choose a reason for hiding this comment

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

Are the functions having the same name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, one with permanent and other with Temporary

Copy link
Contributor

@dilipbiswal dilipbiswal Sep 22, 2019

Choose a reason for hiding this comment

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

@gatorsmile The permanent functions are always shown as database qualified where as the temporary functions are not qualified. Oh.. oops :-) one is test_avg and the other is temp_avg.. . @sandeep-katta can you check, please ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops :), sorry I corrected it

image

@SparkQA
Copy link

SparkQA commented Sep 23, 2019

Test build #111178 has finished for PR 25553 at commit c9b7314.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@sandeep-katta
Copy link
Contributor Author

@dilipbiswal @gatorsmile does any more changes are required for this PR ?


**This page is under construction**
### Description
`DROP FUNCTION` statement drops a temporary or user defined function(UDF). An exception will be thrown if the function does not exist
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Add "The" at the start. Also, space before (UDF). What does 'in the system' mean - can we just drop this or make it more specific?

@SparkQA
Copy link

SparkQA commented Oct 3, 2019

Test build #111739 has finished for PR 25553 at commit 24a13f8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 4, 2019

Test build #111762 has finished for PR 25553 at commit 48f925a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

+-----------+
| function |
+-----------+
| temp_avg |
Copy link
Contributor

Choose a reason for hiding this comment

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

@sandeep-katta Why is it temp_avg here ? Shouldn't it be temporary function test_avg ?

Copy link
Member

Choose a reason for hiding this comment

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

Ping @sandeep-katta to fix that part

+-----------+

-- Drop Temporary function
DROP TEMPORARY FUNCTION IF EXISTS temp_avg;
Copy link
Contributor

Choose a reason for hiding this comment

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

@sandeep-katta same question :-)

+-----------+
| function |
+-----------+
| temp_avg |
Copy link
Member

Choose a reason for hiding this comment

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

Ping @sandeep-katta to fix that part

@SparkQA
Copy link

SparkQA commented Oct 8, 2019

Test build #111895 has finished for PR 25553 at commit 1d3b665.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Oct 9, 2019

Merged to master

@srowen srowen closed this in 69b0cc1 Oct 9, 2019
atronchi pushed a commit to atronchi/spark that referenced this pull request Oct 23, 2019
### What changes were proposed in this pull request?
Add DROP FUNCTION sql description in SQL reference

### Why are the changes needed?
Currently from spark there is no complete sql guide is present, so it is better to document all the sql commands, this jira is sub part of this task.

### Does this PR introduce any user-facing change?
Yes before user cannot find any reference for drop function command in the spark docs.

After Fix:
![image](https://user-images.githubusercontent.com/35216143/66134570-240cd300-e616-11e9-9c78-259c0d355378.png)

![image](https://user-images.githubusercontent.com/35216143/65397825-d059e880-ddd0-11e9-8bd3-a65ccae56063.png)

![image](https://user-images.githubusercontent.com/35216143/66404731-9f032e80-ea06-11e9-8fef-1e266efa4c66.png)

### How was this patch tested?
tested with jekyll build

Closes apache#25553 from sandeep-katta/28797.

Authored-by: sandeep katta <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants