Skip to content

Support DROP FUNCTION#13412

Merged
rongrong merged 6 commits intoprestodb:masterfrom
caithagoras:s3
Dec 3, 2019
Merged

Support DROP FUNCTION#13412
rongrong merged 6 commits intoprestodb:masterfrom
caithagoras:s3

Conversation

@caithagoras
Copy link
Contributor

@caithagoras caithagoras commented Sep 18, 2019

Syntax Reference: #13415

== RELEASE NOTES ==

General
* Add support for ``DROP FUNCTION``.

@caithagoras caithagoras changed the title Drop function syntax Add syntax support for DROP FUNCTION Sep 18, 2019
@rongrong
Copy link
Contributor

Can you add reference to SQL spec if any about the syntax for DROP FUNCTION? Maybe it's better to open an issue first.

@caithagoras caithagoras removed the request for review from rongrong September 18, 2019 19:33
@caithagoras caithagoras force-pushed the s3 branch 3 times, most recently from 41fbb1b to d77bd5f Compare October 8, 2019 08:10
@caithagoras caithagoras force-pushed the s3 branch 6 times, most recently from 56929f8 to b042467 Compare November 4, 2019 17:43
@caithagoras caithagoras changed the title Add syntax support for DROP FUNCTION Support DROP FUNCTION Nov 4, 2019
@caithagoras caithagoras force-pushed the s3 branch 4 times, most recently from 175ed9b to e508028 Compare November 4, 2019 18:20
@caithagoras caithagoras requested a review from rongrong November 4, 2019 20:02
@caithagoras
Copy link
Contributor Author

@rongrong Ready for review

Copy link
Contributor

@rongrong rongrong left a comment

Choose a reason for hiding this comment

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

Mostly looks good. Some comments about error handling.

@caithagoras caithagoras force-pushed the s3 branch 2 times, most recently from a768232 to c4fd96e Compare November 5, 2019 04:48
@caithagoras
Copy link
Contributor Author

caithagoras commented Nov 5, 2019

Also, removed the space between function name and parameter type list when formatting DROP FUNCTION

Copy link
Contributor

Choose a reason for hiding this comment

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

Function not found?

Copy link
Contributor

@rongrong rongrong left a comment

Choose a reason for hiding this comment

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

Looks good! Only one nits.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 7, 2019

CLA Check
The committers are authorized under a signed CLA.

  • ✅ Leiqing Cai (02270700bbd706cb7943a7b37081f8a2a99a205f, 23e9f7cfef7af8ec7885fd2df1fdc68412d7542f, e10ec46e9831173df447b7aaa0c069034b192267, 10f8b29067895b489289e5eda1b1e75800e96c27, 426f085c0deead4707475a580c7b95528bb43fb0)

@caithagoras
Copy link
Contributor Author

Please hold off merging the PR

@caithagoras caithagoras force-pushed the s3 branch 2 times, most recently from 5e4693a to 29852bf Compare December 2, 2019 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants