Skip to content

Renaming to prepare for proc/function support for Pg #1179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from

Conversation

gledis69
Copy link
Contributor

@gledis69 gledis69 commented Feb 3, 2023

Why make this change?

Postgres stored procedures and function are very similar in the way they are defined. The differences as far as our application are concerned are:

  • procedures don't return any data, only functions can return data, but functions can return void as well.
  • procedures are called as CALL <procedure_name>, while functions are called as SELECT * FROM <function_name> or SELECT <function_name>.

Due to these small differences procedures and functions for pg can be handled the same for the most part in our engine so this PR generalizes the nomenclature of any StoredProcedure related structure to DatabaseExecutable to imply both stored procedures and functions. I imagine this will be useful for MsSql and MySql as well when they add function support.

This PR is a prerequisite for a subsequent PR which will add procedure/function support for PG, but I figured this can be reviewed and merged separately since reviewing the proc/func support PR would become too hard due to all the refactoring.

What is this change?

  • Mostly renaming classes, functions, variables which include StoredProcedure to smth more generic DatabaseExecutable.
  • Updating comments to reflect the refactoring.

How was this tested?

  • The CI tests will validate that the refactoring didn't break any of our existing tests

Sample Request(s)

N/A

@Aniruddh25 Aniruddh25 linked an issue Feb 3, 2023 that may be closed by this pull request
@abhishekkumams
Copy link
Contributor

I think once all the conflicts are resolved, it would be easier to review.

@gledis69 gledis69 force-pushed the pre-pg-proc-refacting branch from 245d896 to 94efe5a Compare February 3, 2023 17:08
Aniruddh25
Aniruddh25 previously approved these changes Feb 3, 2023
@gledis69 gledis69 force-pushed the pre-pg-proc-refacting branch from d08890a to 2a60577 Compare February 3, 2023 18:30
@gledis69 gledis69 force-pushed the pre-pg-proc-refacting branch from 4ed97b7 to 308be20 Compare February 3, 2023 19:54
@Aniruddh25 Aniruddh25 dismissed their stale review February 3, 2023 20:35

Need more time to review, given the new function source type.

@seantleonard
Copy link
Contributor

There have been many changes since this was opened. Closing for now and you can re-open with latest changes and conflicts resolve.

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.

[Known Issue] Add support for PostgreSQL Stored-Procedure
5 participants