- Notifications
You must be signed in to change notification settings - Fork6
Pm 2206 master#38
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| CREATESCHEMAIF NOT EXISTS skills; | ||
| CREATESCHEMAIF NOT EXISTS reviews; | ||
| CREATE EXTENSION IF NOT EXISTS fuzzystrmatch WITH SCHEMA skills; |
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.
[💡design]
Thefuzzystrmatch extension is being created in theskills schema. Ensure that this schema is the intended location for this extension, as it might be more conventional to place extensions in thepublic orpg_catalog schemas unless there's a specific reason for this choice.
| CREATE EXTENSION IF NOT EXISTS fuzzystrmatch WITH SCHEMA skills; | ||
| CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA pg_catalog; | ||
| CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA reviews; |
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.
[💡design]
Thepgcrypto extension is being created in thereviews schema. Verify that this schema is the intended location for this extension, as extensions are typically placed in thepublic orpg_catalog schemas unless there's a specific reason for this choice.
| CREATE EXTENSION IF NOT EXISTS fuzzystrmatch WITH SCHEMA skills; | ||
| CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA pg_catalog; | ||
| CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA reviews; | ||
| CREATE EXTENSION IF NOT EXISTS"uuid-ossp" WITH SCHEMA skills; |
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.
[💡design]
Theuuid-ossp extension is being created in theskills schema. Confirm that this schema is the intended location for this extension, as it is more common to place extensions in thepublic orpg_catalog schemas unless there's a specific reason for this choice.
| JOIN pg_namespace nsONns.oid=idx.relnamespace | ||
| WHEREidx.relname='challenge_phase_challenge_open_end_idx' | ||
| ANDns.nspname= challenge_phase_schema | ||
| AND pg_get_indexdef(idx.oid)LIKE'%("challengeId", "isOpen", "scheduledEndDate", "actualEndDate", name)%' |
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.
[maintainability]
Thepg_get_indexdef function is used with aLIKE clause to check index definitions. This approach may be fragile if the index definition changes in the future. Consider using a more robust method to verify index columns, such as queryingpg_index andpg_attribute tables directly.
| AND table_name='Resource' | ||
| ) THEN | ||
| EXECUTE format( | ||
| 'CREATE VIEW %I.%I 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.
[performance]
UsingSELECT DISTINCT can be costly in terms of performance, especially on large datasets. Consider whether the distinctness is necessary or if it can be ensured by other means, such as unique constraints on the underlying table.
| ELSE | ||
| EXECUTE format( | ||
| 'CREATE VIEW %I.%I AS | ||
| SELECT CAST(NULL AS TEXT) AS "challengeId", |
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.
[correctness]
The use ofCAST(NULL AS TEXT) in theELSE branch might lead to confusion or unexpected behavior when querying the view. Consider documenting the intended use case for this scenario or ensuring that downstream consumers of this view handle theseNULL values appropriately.
| termsChallengeTerm[] | ||
| skillsChallengeSkill[] | ||
| auditLogsAuditLog[] | ||
| memberAccessesMemberChallengeAccess[] |
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.
[maintainability]
The addition ofmemberAccesses as a relation in theChallenge model seems correct. However, ensure that theMemberChallengeAccess model is properly populated and maintained to avoid potential issues with orphaned records or incorrect data associations.
| // MemberChallengeAccess view – member/challenge pairs from resources schema | ||
| ////////////////////////////////////////// | ||
| modelMemberChallengeAccess { |
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.
[maintainability]
TheMemberChallengeAccess model lacks auditing fields such ascreatedAt,createdBy,updatedAt, andupdatedBy. Adding these fields would improve traceability and consistency with other models in the schema.
ae6901e intomasterUh oh!
There was an error while loading.Please reload this page.
Performance updates to help with PM-2206