Enhance Oracle-to-PostgreSQL migration skills and documentation (#2284)

- Update migration agent guidelines to prioritize extension tool usage for code migration.
- Refine migration phases with detailed steps for pre-migration review and schema migration.
- Add new reviewing skill references for PostgreSQL materialized view refresh and UNION ALL planner risks.
- Ensure consistency in collation handling and testing strategies across skills.

Co-authored-by: TCPrimedPaul <paul.delannoy@tc.gc.ca>

skills/creating-oracle-to-postgres-migration-integration-tests/SKILL.md

@@ -36,17 +36,22 @@ Scope to the target project only. List data access methods that interact with th
 - Follow seed file location and naming conventions from the existing project.
 - Reuse existing seed files when possible.
 - Avoid `TRUNCATE TABLE` — keep existing database data intact.
+- Assume existing business rows and lookup rows are already present; add only minimal, collision-safe seed records needed for the scenario.
 - Do not commit seed data; tests run in transactions that roll back.
 - Ensure seed data does not conflict with other tests.
 - Load and verify seed data before assertions depend on it.
+- Create or reuse a test `LookupConstants` class for stable lookup IDs/codes used across seed builders and assertions.
 
 **Step 4: Write test cases**
 
 - Inherit from the base test class to get automatic transaction create/rollback.
+- Ensure each database-touching method in scope has at least one integration test (or multiple tests for higher-risk behavior branches).
 - Assert logical outputs (rows, columns, counts, error types), not platform-specific messages.
 - Assert specific expected values — never assert that a value is merely non-null or non-empty when a concrete value is available from seed data.
 - Avoid testing code paths that do not exist or asserting behavior that cannot occur.
 - Avoid redundant assertions across tests targeting the same method.
+- For text parameters, include both empty-string and `NULL`/missing input coverage where applicable.
+- For datetime behavior, include explicit timezone-sensitive assertions when methods write/read `timestamp without time zone` or `timestamp(0)` targets.
 
 **Step 5: Review determinism**
 
@@ -58,3 +63,4 @@ Re-examine every assertion against non-null values. Confirm each is deterministi
 - **DB-agnostic assertions** — no platform-specific error messages or syntax in assertions.
 - **Seed only against Oracle** — test project will be migrated to PostgreSQL later.
 - **Scoped to one project** — do not create tests for artifacts outside the target project.
+- **Preserve existing data** — never rewrite or wipe pre-existing business or lookup rows.

skills/migrating-oracle-to-postgres-stored-procedures/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: migrating-oracle-to-postgres-stored-procedures
-description: 'Migrates Oracle PL/SQL stored procedures to PostgreSQL PL/pgSQL. Translates Oracle-specific syntax, preserves method signatures and type-anchored parameters, leverages orafce where appropriate, and applies COLLATE "C" for Oracle-compatible text sorting. Use when converting Oracle stored procedures or functions to PostgreSQL equivalents during a database migration.'
+description: 'Migrates Oracle PL/SQL stored procedures to PostgreSQL PL/pgSQL. Translates Oracle-specific syntax, preserves method signatures and type-anchored parameters, leverages orafce where appropriate, and applies explicit collation mapping (`COLLATE "C"` only when appropriate, locale collations when required). Use when converting Oracle stored procedures or functions to PostgreSQL equivalents during a database migration.'
 ---
 
 # Migrating Stored Procedures from Oracle to PostgreSQL
@@ -32,7 +32,11 @@ Apply these translation rules:
 - Do not prefix object names with schema names unless already present in the Oracle source.
 - Leave exception handling and rollback logic unchanged.
 - Do not generate `COMMENT` or `GRANT` statements.
-- Use `COLLATE "C"` when ordering by text fields for Oracle-compatible sorting.
+- Apply collation intentionally when ordering text:
+  - Use `COLLATE "C"` only when Oracle-compatible binary ordering is required and no other sort order is specified.
+  - If Oracle used explicit linguistic sorting (for example `NLS_SORT = French`), map to an explicit PostgreSQL locale collation instead of `"C"`.
+  - Use `SELECT collname, collprovider, collcollate, collctype FROM pg_collation ORDER BY collname;` to discover collations in the target environment.
+- Treat `UNION ALL` as a review checkpoint. Validate plan quality per branch and restructure if combined-branch planning causes regressions (for example, unexpected sequential scans on large tables).
 - Leverage the `orafce` extension when it improves clarity or fidelity.
 
 Consult the PostgreSQL table/view definitions at `.github/oracle-to-postgres-migration/DDL/Postgres/Tables and Views/` for target schema details.

skills/planning-oracle-to-postgres-migration-integration-testing/SKILL.md

@@ -31,6 +31,12 @@ Write a markdown plan covering:
 - Recommended test cases per artifact
 - Seed data requirements
 - Known Oracle→PostgreSQL behavioral differences to validate
+- Coverage mapping that ensures every database touchpoint has at least one test case (or a justified set of cases for high-risk methods)
+
+When defining recommended test cases, explicitly include:
+- Text parameter behavior for both empty string and `NULL`/missing values.
+- Datetime/timezone assertions, including round-trip and comparison behavior.
+- Cases where destination columns use `timestamp without time zone` or `timestamp(0)`, with explicit timezone-application expectations.
 
 ## Output
 

skills/reviewing-oracle-to-postgres-migration/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: reviewing-oracle-to-postgres-migration
-description: 'Identifies Oracle-to-PostgreSQL migration risks by cross-referencing code against known behavioral differences (empty strings, refcursors, type coercion, sorting, timestamps, concurrent transactions, etc.). Use when planning a database migration, reviewing migration artifacts, or validating that integration tests cover Oracle/PostgreSQL differences.'
+description: 'Identifies Oracle-to-PostgreSQL migration risks by cross-referencing code against known behavioral differences (empty strings, refcursors, type coercion, sorting/collations, UNION ALL planner risks, materialized-view refresh requirements, timestamps, concurrent transactions, etc.). Use when planning a database migration, reviewing migration artifacts, or validating that integration tests cover Oracle/PostgreSQL differences.'
 ---
 
 # Oracle-to-PostgreSQL Database Migration
@@ -60,7 +60,7 @@ For each reference in [references/REFERENCE.md](references/REFERENCE.md), confir
 
 **Step 3: Verify integration test coverage**
 
-Confirm tests exercise both the happy path and the failure scenarios highlighted in applicable insights (exceptions, sorting, refcursor consumption, concurrent transactions, timestamps, etc.).
+Confirm tests exercise both the happy path and the failure scenarios highlighted in applicable insights (exceptions, sorting, `UNION ALL` behavior/performance risks, refcursor consumption, concurrent transactions, timestamps, materialized-view freshness, etc.).
 
 **Step 4: Gate the result**
 

skills/reviewing-oracle-to-postgres-migration/references/REFERENCE.md

@@ -8,6 +8,8 @@
 | [oracle-to-postgres-sorting.md](oracle-to-postgres-sorting.md) | How to preserve Oracle-like ordering in PostgreSQL using COLLATE "C" and DISTINCT wrapper patterns. |
 | [oracle-to-postgres-to-char-numeric.md](oracle-to-postgres-to-char-numeric.md) | Oracle allows TO_CHAR(numeric) without format; PostgreSQL requires format string—use CAST(numeric AS TEXT) instead. |
 | [oracle-to-postgres-type-coercion.md](oracle-to-postgres-type-coercion.md) | PostgreSQL strict type checks vs. Oracle implicit coercion—fix comparison errors by quoting or casting literals. |
+| [postgres-union-all-planner.md](postgres-union-all-planner.md) | UNION ALL branches can produce poor plans when predicate pushdown is limited—review plans and split or reshape queries when needed. |
+| [postgres-materialized-view-refresh.md](postgres-materialized-view-refresh.md) | Materialized views are not auto-refreshed after base-table changes—application or jobs must explicitly refresh them. |
 | [postgres-concurrent-transactions.md](postgres-concurrent-transactions.md) | PostgreSQL allows only one active command per connection—materialize results or use separate connections to avoid concurrent operation errors. |
 | [postgres-refcursor-handling.md](postgres-refcursor-handling.md) | Differences in refcursor handling; PostgreSQL requires fetching by cursor name—C# patterns to unwrap and read results. |
 | [oracle-to-postgres-timestamp-timezone.md](oracle-to-postgres-timestamp-timezone.md) | CURRENT_TIMESTAMP / NOW() return UTC-normalised timestamptz in PostgreSQL; Npgsql surfaces DateTime.Kind=Unspecified—force UTC at connection open and in application code. |

skills/reviewing-oracle-to-postgres-migration/references/oracle-to-postgres-sorting.md

@@ -3,13 +3,14 @@
 Purpose: Preserve Oracle-like sorting semantics when moving queries to PostgreSQL.
 
 ## Key points
-- Oracle often treats plain `ORDER BY` as binary/byte-wise, giving case-insensitive ordering for ASCII.
-- PostgreSQL defaults differ; to match Oracle behavior, use `COLLATE "C"` on sort expressions.
+- Oracle and PostgreSQL default collations can differ significantly.
+- Use `COLLATE "C"` only when you explicitly need Oracle-like binary ordering and no different sort rule is requested.
+- If Oracle uses explicit linguistic ordering (for example `NLS_SORT = French`), map to an explicit PostgreSQL locale collation instead of forcing `"C"`.
 
 ## 1) Standard `SELECT … ORDER BY`
 **Goal:** Keep Oracle-style ordering.
 
-**Pattern:**
+**Pattern (only when Oracle-compatible binary ordering is required):**
 ```sql
 SELECT col1
 FROM your_table
@@ -17,9 +18,27 @@ ORDER BY col1 COLLATE "C";
 ```
 
 **Notes:**
-- Apply `COLLATE "C"` to each sort expression that must mimic Oracle.
+- Apply `COLLATE "C"` only to sort expressions that must mimic Oracle binary ordering.
 - Works with ascending/descending and multi-column sorts, e.g. `ORDER BY col1 COLLATE "C", col2 COLLATE "C" DESC`.
 
+## 1b) Locale-aware ordering (when Oracle used NLS_SORT)
+
+If Oracle used locale-specific sorting such as:
+```sql
+ORDER BY nlssort(Externalusers.UserID, 'NLS_SORT = French')
+```
+map to an explicit PostgreSQL collation, for example:
+```sql
+ORDER BY Externalusers.UserID COLLATE "ca_FR.utf-8"
+```
+
+Use a collation that exists in the target environment. Discover available collations with:
+```sql
+SELECT collname, collprovider, collcollate, collctype
+FROM pg_collation
+ORDER BY collname;
+```
+
 ## 2) `SELECT DISTINCT … ORDER BY`
 **Issue:** PostgreSQL enforces that `ORDER BY` expressions appear in the `SELECT` list for `DISTINCT`, raising:
 `Npgsql.PostgresException: 42P10: for SELECT DISTINCT, ORDER BY expressions must appear in select list`
@@ -38,14 +57,14 @@ ORDER BY col2 COLLATE "C";
 
 **Why:**
 - The inner query performs the `DISTINCT` projection.
-- The outer query safely orders the result set and adds `COLLATE "C"` to align with Oracle sorting.
+- The outer query safely orders the result set and adds an explicit collation where needed to align with Oracle sorting.
 
 **Tips:**
 - Ensure any columns used in the outer `ORDER BY` are included in the inner projection.
 - For multi-column sorts, collate each relevant expression: `ORDER BY col2 COLLATE "C", col3 COLLATE "C" DESC`.
 
 ## Validation checklist
-- [ ] Added `COLLATE "C"` to every `ORDER BY` that should follow Oracle sorting rules.
+- [ ] Applied explicit collation only where required (`"C"` for Oracle-style binary ordering, locale collation for linguistic ordering).
 - [ ] For `DISTINCT` queries, wrapped the projection and sorted in the outer query.
 - [ ] Confirmed ordered columns are present in the inner projection.
 - [ ] Re-ran tests or representative queries to verify ordering matches Oracle outputs.

skills/reviewing-oracle-to-postgres-migration/references/postgres-materialized-view-refresh.md

@@ -0,0 +1,35 @@
+# PostgreSQL Materialized View Refresh Guide
+
+Purpose: Ensure migrated applications keep materialized views current after base-table changes.
+
+## Problem
+
+PostgreSQL materialized views are static snapshots. Updates to source tables do **not** automatically refresh dependent materialized views.
+
+## Migration risk
+
+- Oracle-era assumptions that derived data updates immediately may no longer hold.
+- Read paths can return stale rows unless refresh timing is explicitly managed.
+- Integration tests may pass once and then fail intermittently if refresh sequencing is not deterministic.
+
+## Required review item
+
+For every migrated path that writes to tables feeding a materialized view, verify the application workflow includes an explicit refresh strategy.
+
+## Refresh patterns
+
+- Immediate refresh in the write workflow when freshness is required:
+  ```sql
+  REFRESH MATERIALIZED VIEW my_view;
+  ```
+- Concurrent refresh (when supported and indexed) to reduce read blocking:
+  ```sql
+  REFRESH MATERIALIZED VIEW CONCURRENTLY my_view;
+  ```
+- Scheduled/batch refresh when stale windows are acceptable.
+
+## Integration-test expectations
+
+- [ ] Tests that modify source tables assert materialized-view contents only after the intended refresh action.
+- [ ] Tests assert stale behavior before refresh when applicable.
+- [ ] Tests document whether freshness is immediate or eventual for each affected feature.

skills/reviewing-oracle-to-postgres-migration/references/postgres-union-all-planner.md

@@ -0,0 +1,31 @@
+# PostgreSQL UNION ALL Planner Risk Guide
+
+Purpose: Avoid regressions where migrated `UNION ALL` queries run much slower in PostgreSQL than expected.
+
+## Problem
+
+`UNION ALL` keeps duplicate rows and combines branch outputs directly, but PostgreSQL does not always optimize each branch as aggressively as isolated queries. In large datasets this can produce poor plans (for example full scans where index-based plans are expected).
+
+## Why it happens
+
+- Predicate pushdown through `UNION ALL` branches can be limited depending on query shape.
+- Cardinality estimates across branches can be skewed.
+- Branch-local indexes may not be chosen when the optimizer evaluates the combined query.
+
+## Review checklist
+
+- [ ] Compare `EXPLAIN (ANALYZE, BUFFERS)` plans for the combined `UNION ALL` query and branch-isolated variants.
+- [ ] Confirm branch predicates are explicit and not hidden inside non-sargable expressions.
+- [ ] Check for unexpected sequential scans on large tables in either branch.
+- [ ] Verify indexes exist for each branch's filter and join predicates.
+
+## Mitigation patterns
+
+1. Test each branch independently to verify expected index usage.
+2. Push filters down into each branch instead of only filtering in the outer query.
+3. If plan quality remains poor, split the query into two separately executed statements and combine results in application code.
+4. Consider materializing branch results in temporary/intermediate structures only when measurement confirms benefit.
+
+## Validation note
+
+Treat `UNION ALL` performance behavior as a migration review item even when functional test results match Oracle.