diff --git a/PLANS/2026-01-06-search-criteria-extraction-design.md b/PLANS/2026-01-06-search-criteria-extraction-design.md index 95b2ca8..fb294d5 100644 --- a/PLANS/2026-01-06-search-criteria-extraction-design.md +++ b/PLANS/2026-01-06-search-criteria-extraction-design.md @@ -16,7 +16,19 @@ Create SQL Server functions to extract values from the `Search.Criteria` JSON co ### SQL Server Version - **Target:** SQL Server 2022 -- **JSON Functions:** `OPENJSON()`, `JSON_VALUE()`, `ISJSON()` +- **JSON Functions:** `OPENJSON()`, `JSON_VALUE()`, `ISJSON()`, `TRY_CONVERT()` + +### Design Decision: Inline TVFs + Stored Procedure Validation + +**Codex Review Findings:** +1. **THROW cannot be used in UDFs** - SQL Server restriction applies to all function types +2. **Multi-statement TVFs have poor performance** - Table variables lack statistics, causing bad cardinality estimates +3. **Inline TVFs are optimal** - Can be inlined into query plans with proper optimization + +**Chosen Pattern:** +- **Inline TVFs** for extraction (performance-critical, used in query builder) +- **Validation stored procedure** for error handling when needed +- **C# validates search exists** before calling query builder (defense in depth) ### Function Types @@ -27,7 +39,7 @@ Create SQL Server functions to extract values from the `Search.Criteria` JSON co | `dbo.fn_GetSearchMaximumDt` | `DATETIME2(7)` | `$.MaximumDt` | | `dbo.fn_GetSearchExtractMisData` | `BIT` | `$.ExtractMisData` | -**Simple Table Functions (5):** +**Simple Table Functions (5) - Inline TVFs:** | Function | Returns | JSON Path | |----------|---------|-----------| | `dbo.fn_GetSearchWorkOrders` | `TABLE(WorkOrderNumber BIGINT)` | `$.WorkOrderNumbers` | @@ -36,15 +48,31 @@ Create SQL Server functions to extract values from the `Search.Criteria` JSON co | `dbo.fn_GetSearchWorkCenters` | `TABLE(Code VARCHAR(12))` | `$.WorkCenters` | | `dbo.fn_GetSearchOperatorIDs` | `TABLE(OperatorID VARCHAR(128))` | `$.OperatorIDs` | -**Complex Table Functions (2):** +**Complex Table Functions (2) - Inline TVFs with OPENJSON...WITH:** | Function | Returns | JSON Path | |----------|---------|-----------| | `dbo.fn_GetSearchComponentLots` | `TABLE(LotNumber VARCHAR(30), ItemNumber VARCHAR(128))` | `$.ComponentLotNumbers[*]` | | `dbo.fn_GetSearchPartOperations` | `TABLE(ItemNumber VARCHAR(128), OperationNumber VARCHAR(10), MisNumber VARCHAR(10), MisRevision VARCHAR(10))` | `$.PartOperations[*]` | -### Error Handling +**Validation Stored Procedure (1):** +| Procedure | Purpose | +|-----------|---------| +| `dbo.usp_ValidateSearchCriteria` | Validates search exists and has valid JSON, throws errors | -All functions validate inputs and throw errors for invalid conditions: +### Error Handling Strategy + +**Two-tier approach:** + +1. **Inline TVFs (graceful):** Return empty results for all edge cases + - Search not found → empty + - Criteria NULL/empty → empty + - Invalid JSON → empty (ISJSON guard) + - Property missing → empty + - Bad data types → filtered out (TRY_CONVERT) + +2. **Validation Procedure (strict):** Throws errors for invalid conditions + - Used when explicit validation needed + - Called before query execution if strict mode required | Error Code | Condition | Message Pattern | |------------|-----------|-----------------| @@ -52,55 +80,100 @@ All functions validate inputs and throw errors for invalid conditions: | 50002 | Criteria is NULL or empty | `Search ID {id} has no criteria` | | 50003 | Criteria is invalid JSON | `Search ID {id} has invalid JSON` | -**Normal empty results (no error):** -- JSON valid but property missing -- JSON valid, property is `[]` or `null` - -### Multi-Statement TVF Pattern - -Table functions use multi-statement TVFs for proper error handling: +### Inline TVF Pattern (Simple Arrays) ```sql CREATE FUNCTION dbo.fn_GetSearchWorkOrders(@SearchId INT) -RETURNS @Results TABLE (WorkOrderNumber BIGINT NOT NULL) +RETURNS TABLE +AS +RETURN +( + SELECT TRY_CONVERT(BIGINT, j.[value]) AS WorkOrderNumber + FROM dbo.Search s + CROSS APPLY OPENJSON(s.Criteria, '$.WorkOrderNumbers') j + WHERE s.ID = @SearchId + AND s.Criteria IS NOT NULL + AND ISJSON(s.Criteria) = 1 + AND TRY_CONVERT(BIGINT, j.[value]) IS NOT NULL +); +GO +``` + +### Inline TVF Pattern (Complex Objects with OPENJSON...WITH) + +```sql +CREATE FUNCTION dbo.fn_GetSearchComponentLots(@SearchId INT) +RETURNS TABLE +AS +RETURN +( + SELECT j.LotNumber, j.ItemNumber + FROM dbo.Search s + CROSS APPLY OPENJSON(s.Criteria, '$.ComponentLotNumbers') + WITH ( + LotNumber VARCHAR(30) '$.LotNumber', + ItemNumber VARCHAR(128) '$.ItemNumber' + ) j + WHERE s.ID = @SearchId + AND s.Criteria IS NOT NULL + AND ISJSON(s.Criteria) = 1 +); +GO +``` + +### Scalar Function Pattern + +```sql +CREATE FUNCTION dbo.fn_GetSearchMinimumDt(@SearchId INT) +RETURNS DATETIME2(7) AS BEGIN + DECLARE @Result DATETIME2(7); + + SELECT @Result = TRY_CONVERT(DATETIME2(7), JSON_VALUE(s.Criteria, '$.MinimumDt')) + FROM dbo.Search s + WHERE s.ID = @SearchId + AND s.Criteria IS NOT NULL + AND ISJSON(s.Criteria) = 1; + + RETURN @Result; +END +GO +``` + +### Validation Stored Procedure + +```sql +CREATE PROCEDURE dbo.usp_ValidateSearchCriteria(@SearchId INT) +AS +BEGIN + SET NOCOUNT ON; DECLARE @Criteria VARCHAR(MAX); DECLARE @ErrorMsg NVARCHAR(400); - -- Get criteria SELECT @Criteria = Criteria FROM dbo.Search WHERE ID = @SearchId; - -- Validate search exists IF @@ROWCOUNT = 0 BEGIN SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' not found'); THROW 50001, @ErrorMsg, 1; END - -- Validate criteria not null/empty IF @Criteria IS NULL OR @Criteria = '' BEGIN SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has no criteria'); THROW 50002, @ErrorMsg, 1; END - -- Validate JSON IF ISJSON(@Criteria) = 0 BEGIN SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has invalid JSON'); THROW 50003, @ErrorMsg, 1; END - - -- Extract values (returns empty if property missing/null/empty array) - INSERT INTO @Results (WorkOrderNumber) - SELECT CAST(value AS BIGINT) - FROM OPENJSON(@Criteria, '$.WorkOrderNumbers'); - - RETURN; END +GO ``` ## Migration Scripts @@ -108,21 +181,24 @@ END ### New Scripts **045_CreateScalarExtractionFunctions.sql** -- `fn_GetSearchMinimumDt` -- `fn_GetSearchMaximumDt` -- `fn_GetSearchExtractMisData` +- `fn_GetSearchMinimumDt` (scalar) +- `fn_GetSearchMaximumDt` (scalar) +- `fn_GetSearchExtractMisData` (scalar) -**046_CreateSimpleTableFunctions.sql** +**046_CreateSimpleTableFunctions.sql** (inline TVFs) - `fn_GetSearchWorkOrders` - `fn_GetSearchItemNumbers` - `fn_GetSearchProfitCenters` - `fn_GetSearchWorkCenters` - `fn_GetSearchOperatorIDs` -**047_CreateComplexTableFunctions.sql** +**047_CreateComplexTableFunctions.sql** (inline TVFs with OPENJSON...WITH) - `fn_GetSearchComponentLots` - `fn_GetSearchPartOperations` +**048_CreateValidateSearchCriteriaProcedure.sql** +- `usp_ValidateSearchCriteria` (stored procedure with THROW) + ### Scripts to Delete Remove obsolete Table Type scripts: @@ -184,13 +260,34 @@ tests/JdeScoping.Database.Tests/ │ ├── ScalarFunctionTests.cs │ ├── SimpleTableFunctionTests.cs │ └── ComplexTableFunctionTests.cs +└── Procedures/ + └── ValidateSearchCriteriaProcedureTests.cs ``` ### Test Categories -**Happy Path:** Valid JSON, correct extraction, empty arrays -**Error Tests:** Non-existent SearchId, NULL criteria, invalid JSON -**Edge Cases:** Large arrays, special characters, NULL values in arrays, Unicode +**Inline TVF Tests (graceful behavior):** +- Valid JSON → correct extraction +- Empty array → empty result +- Missing property → empty result +- Search not found → empty result +- NULL criteria → empty result +- Invalid JSON → empty result +- Bad data types → filtered out (TRY_CONVERT) + +**Validation Procedure Tests (strict behavior):** +- Valid search → no error, completes successfully +- Search not found → throws error 50001 +- NULL criteria → throws error 50002 +- Empty criteria → throws error 50002 +- Invalid JSON → throws error 50003 + +**Edge Cases:** +- Large arrays (1000+ items) +- Special characters in string values +- NULL values within arrays +- Unicode characters +- Nested JSON objects ### Test Infrastructure @@ -210,48 +307,51 @@ Share with `Api.IntegrationTests` via `TestWebApplicationFactory` pattern. ## Implementation Order -### Phase 1: SQL Functions -1. Create `045_CreateScalarExtractionFunctions.sql` -2. Create `046_CreateSimpleTableFunctions.sql` -3. Create `047_CreateComplexTableFunctions.sql` -4. Delete obsolete Table Type scripts (033-039) -5. Renumber remaining scripts (040-044 → 033-037) +### Phase 1: SQL Functions & Procedure +1. Create `045_CreateScalarExtractionFunctions.sql` (3 scalar functions) +2. Create `046_CreateSimpleTableFunctions.sql` (5 inline TVFs) +3. Create `047_CreateComplexTableFunctions.sql` (2 inline TVFs with OPENJSON...WITH) +4. Create `048_CreateValidateSearchCriteriaProcedure.sql` (validation procedure) +5. Delete obsolete Table Type scripts (033-039) +6. Renumber remaining scripts (040-044 → 033-037) ### Phase 2: Database Tests -6. Set up `DatabaseTestBase.cs` -7. Write `ScalarFunctionTests.cs` -8. Write `SimpleTableFunctionTests.cs` -9. Write `ComplexTableFunctionTests.cs` -10. Verify all tests pass +7. Set up `DatabaseTestBase.cs` +8. Write `ScalarFunctionTests.cs` +9. Write `SimpleTableFunctionTests.cs` +10. Write `ComplexTableFunctionTests.cs` +11. Write `ValidateSearchCriteriaProcedureTests.cs` +12. Verify all tests pass ### Phase 3: C# Refactor -11. Update `ISearchQueryBuilder` interface -12. Update `SqlKataSearchQueryBuilder` -13. Update `SearchProcessor` -14. Simplify `SearchModel` -15. Delete `TableValuedParameterExtensions.cs` -16. Delete `FilterEntries/*.cs` -17. Update/delete related tests +13. Update `ISearchQueryBuilder` interface +14. Update `SqlKataSearchQueryBuilder` +15. Update `SearchProcessor` +16. Simplify `SearchModel` +17. Delete `TableValuedParameterExtensions.cs` +18. Delete `FilterEntries/*.cs` +19. Update/delete related tests ### Phase 4: Integration & Verification -18. Run full test suite -19. Fix broken tests -20. Manual end-to-end verification +20. Run full test suite +21. Fix broken tests +22. Manual end-to-end verification ### Phase 5: Documentation -21. Update OpenSpec specifications -22. Update architecture documentation -23. Add Database.Tests documentation -24. Create ExtractionFunctions.md reference +23. Update OpenSpec specifications +24. Update architecture documentation +25. Add Database.Tests documentation +26. Create ExtractionFunctions.md reference ## Acceptance Criteria -- [ ] All 11 SQL functions created and working -- [ ] Error codes 50001/50002/50003 thrown for invalid inputs -- [ ] Empty results returned for missing/null/empty properties -- [ ] Table Type scripts removed -- [ ] C# TVP code removed +- [ ] All 11 SQL inline TVFs/scalar functions created and working +- [ ] Validation stored procedure created with THROW for errors +- [ ] Inline TVFs return empty results for all edge cases (graceful) +- [ ] Validation procedure throws 50001/50002/50003 for invalid inputs (strict) +- [ ] Table Type scripts (033-039) removed +- [ ] C# TVP code removed (TableValuedParameterExtensions, FilterEntries) - [ ] Query builder uses SearchId parameter only -- [ ] Database.Tests passing (all categories) +- [ ] Database.Tests passing (functions + procedure) - [ ] Existing integration tests passing - [ ] Documentation updated