-
Notifications
You must be signed in to change notification settings - Fork 19
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
MattT/APPEALS-64534: Create PL/pgSQL Function to Return a Table of f_vacols_brieff Records Given a String Containing vacols_ids #23484
Conversation
Code Climate has analyzed commit 069432c and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
@@ -0,0 +1,87 @@ | |||
CREATE TYPE brieff_record AS ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I broke this out to allow CI to run properly. Unfortunately, custom types aren't registered in the schema.rb file and aren't automatically loaded by our build commands.
|
||
RETURN QUERY | ||
EXECUTE format( | ||
'SELECT * FROM f_vacols_brieff WHERE bfkey IN (%s)', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By passing in IDs as constants to this query it avoids performing a full foreign table scan and the query is pushed down to the remote database.
This function has been tested in Prodtest and has been confirmed to complete execution within a few seconds. This is down from a couple of hours. 🤕 |
@@ -0,0 +1,101 @@ | |||
class CreateBrieffReturnType < ActiveRecord::Migration[6.1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this table not need to exist within the schema?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a custom type, and those don't appear to be recorded in the schema.rb file. Kind of a bummer...
@@ -0,0 +1,52 @@ | |||
# frozen_string_literal: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this spec file? I don't see any expectations or assertions in it so confused as to what it's testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just a shared_context that can be "imported" into other specs. I found myself needing to stage all of the same data a second time and decided to extract it out.
include_context "Legacy appeals that may or may not appear in the NHQ" |
caseflow/spec/db/functions/gather_vacols_ids_of_hearing_schedulable_legacy_appeals_spec.rb
Line 4 in e6e7d2f
include_context "Legacy appeals that may or may not appear in the NHQ" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This context will likely be used a few more times.
3f1c45b
into
feature/APPEALS-57706
Resolves APPEALS-64534
Description
Creates a PL/pgSQL function that within itself invokes
gather_vacols_ids_of_hearing_schedulable_legacy_appeals()
in order to identify which vacols_ids should be utilized to pull BRIEFF records. This is done so that the IDs will be constantized and the query pushed down to the remote database whenever querying the foreign table.Acceptance Criteria
bf_key
values are in the result of calling thegather_vacols_ids_of_hearing_schedulable_legacy*appeals
function (see APPEALS-64451)Testing Plan
Backend
Database Changes
Only for Schema Changes
created_at
,updated_at
) for new tablesCaseflow::Migration
, especially when adding indexes (useadd_safe_index
) (see Writing DB migrations)migrate:rollback
works as desired (change
supported functions)make check-fks
; add any missing foreign keys or add toconfig/initializers/immigrant.rb
(see Record associations and Foreign Keys)belongs_to
for associations to enable the schema diagrams to be automatically updatedBest practices
Code Documentation Updates
Tests
Test Coverage
Did you include any test coverage for your code? Check below:
Code Climate
Your code does not add any new code climate offenses? If so why?
Monitoring, Logging, Auditing, Error, and Exception Handling Checklist
Monitoring
Logging
Auditing
Error Handling
Exception Handling