Skip to content
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

Bundled some changes to SQL and functions. #19

Closed
wants to merge 5 commits into from
Closed

Bundled some changes to SQL and functions. #19

wants to merge 5 commits into from

Conversation

bricklen
Copy link

  • Converted a couple plpgsql functions to SQL functions, for a couple reasons: to allow the planner to optimize them in query plans, and to remove the cost of invoking the plpgsql procedural engine unnecessarily (the functions didn't need the full power of a procedural language, SQL was sufficient).
  • Added the "-X" flag to several invocations of psql, so as to not source any customer .psqlrc the user might have set up. By sourcing the .psqlrc, there is a chance of unexpected results occurring when the scripts are executed (eg. pager settings could cause the terminal to hang at a prompt).
  • Replaced several examples of assignment using "=" with ":=".

bricklen and others added 5 commits April 10, 2017 14:40
* Fixed bug in "table_oid::TEXT" section of the "TABLE_NAME" code. Executing `SELECT create_hypertable('my_table', 'time');` failed with `ERROR:  Cannot process index with definition (no index name match)...`.
* Replaced "=" with the preferred ":=" assignment operator. Both are allowed, but the latter is less likely to cause subtle bugs due to equality vs assignment (I've run into it before).
* Qualified system relations with pg_catalog schema so there is no ambiguity.
* Dropped the "CREATE" keyword from the "replace()" function because it was failing to cover the case where an index had a definition like "CREATE UNIQUE INDEX".
* Updated some comments.
* Revised variable names to have a "v_" prefix to reduce the chance of a name collision between objects in a function (I've hit this in production before).
@akulkarni
Copy link
Member

Wrote this on your other PR, but also copying here:

Thanks for the PR. We're in the process of getting ourselves ready to accept external contributions (e.g., putting together a style guide). Should have that ready soon.

Also, noticed that the tests are failing. Think you need to change the expected test result to work with your changes?

@bricklen
Copy link
Author

Closing in favour of a new, leaner version. The extra changes from this PR can be revisited at a later date.

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.

2 participants