397b339c86
Codex review findings applied: - Use CTE pattern to pre-filter valid JSON before OPENJSON - Use OPENJSON...WITH for type-safe extraction (avoids double TRY_CONVERT) - Keep script gaps instead of renumbering (prevents migration drift) - Add xUnit Collection for test isolation (prevents parallel execution issues)
357 lines
12 KiB
Markdown
357 lines
12 KiB
Markdown
# Search Criteria SQL Extraction Functions - Design
|
|
|
|
## Purpose
|
|
|
|
Create SQL Server functions to extract values from the `Search.Criteria` JSON column, eliminating the need for C# to deserialize criteria and pass Table-Valued Parameters to SQL Server. The query builder will generate SQL that extracts filter values directly from the database.
|
|
|
|
## Goals
|
|
|
|
1. Create 11 SQL functions to extract scalar and table values from SearchCriteria JSON
|
|
2. Remove Table Type dependencies (7 TVP types)
|
|
3. Simplify C# query generation to pass only SearchId
|
|
4. Add comprehensive Database.Tests for the new functions
|
|
5. Update documentation and specifications
|
|
|
|
## Architecture
|
|
|
|
### SQL Server Version
|
|
- **Target:** SQL Server 2022
|
|
- **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
|
|
|
|
**Scalar Functions (3):**
|
|
| Function | Returns | JSON Path |
|
|
|----------|---------|-----------|
|
|
| `dbo.fn_GetSearchMinimumDt` | `DATETIME2(7)` | `$.MinimumDt` |
|
|
| `dbo.fn_GetSearchMaximumDt` | `DATETIME2(7)` | `$.MaximumDt` |
|
|
| `dbo.fn_GetSearchExtractMisData` | `BIT` | `$.ExtractMisData` |
|
|
|
|
**Simple Table Functions (5) - Inline TVFs:**
|
|
| Function | Returns | JSON Path |
|
|
|----------|---------|-----------|
|
|
| `dbo.fn_GetSearchWorkOrders` | `TABLE(WorkOrderNumber BIGINT)` | `$.WorkOrderNumbers` |
|
|
| `dbo.fn_GetSearchItemNumbers` | `TABLE(ItemNumber VARCHAR(128))` | `$.ItemNumbers` |
|
|
| `dbo.fn_GetSearchProfitCenters` | `TABLE(Code VARCHAR(12))` | `$.ProfitCenters` |
|
|
| `dbo.fn_GetSearchWorkCenters` | `TABLE(Code VARCHAR(12))` | `$.WorkCenters` |
|
|
| `dbo.fn_GetSearchOperatorIDs` | `TABLE(OperatorID VARCHAR(128))` | `$.OperatorIDs` |
|
|
|
|
**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[*]` |
|
|
|
|
**Validation Stored Procedure (1):**
|
|
| Procedure | Purpose |
|
|
|-----------|---------|
|
|
| `dbo.usp_ValidateSearchCriteria` | Validates search exists and has valid JSON, throws errors |
|
|
|
|
### 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 |
|
|
|------------|-----------|-----------------|
|
|
| 50001 | SearchId not found | `Search ID {id} not found` |
|
|
| 50002 | Criteria is NULL or empty | `Search ID {id} has no criteria` |
|
|
| 50003 | Criteria is invalid JSON | `Search ID {id} has invalid JSON` |
|
|
|
|
### Inline TVF Pattern (Simple Arrays)
|
|
|
|
```sql
|
|
CREATE FUNCTION dbo.fn_GetSearchWorkOrders(@SearchId INT)
|
|
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);
|
|
|
|
SELECT @Criteria = Criteria
|
|
FROM dbo.Search
|
|
WHERE ID = @SearchId;
|
|
|
|
IF @@ROWCOUNT = 0
|
|
BEGIN
|
|
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' not found');
|
|
THROW 50001, @ErrorMsg, 1;
|
|
END
|
|
|
|
IF @Criteria IS NULL OR @Criteria = ''
|
|
BEGIN
|
|
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has no criteria');
|
|
THROW 50002, @ErrorMsg, 1;
|
|
END
|
|
|
|
IF ISJSON(@Criteria) = 0
|
|
BEGIN
|
|
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has invalid JSON');
|
|
THROW 50003, @ErrorMsg, 1;
|
|
END
|
|
END
|
|
GO
|
|
```
|
|
|
|
## Migration Scripts
|
|
|
|
### New Scripts
|
|
|
|
**045_CreateScalarExtractionFunctions.sql**
|
|
- `fn_GetSearchMinimumDt` (scalar)
|
|
- `fn_GetSearchMaximumDt` (scalar)
|
|
- `fn_GetSearchExtractMisData` (scalar)
|
|
|
|
**046_CreateSimpleTableFunctions.sql** (inline TVFs)
|
|
- `fn_GetSearchWorkOrders`
|
|
- `fn_GetSearchItemNumbers`
|
|
- `fn_GetSearchProfitCenters`
|
|
- `fn_GetSearchWorkCenters`
|
|
- `fn_GetSearchOperatorIDs`
|
|
|
|
**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:
|
|
- `033_CreateWorkOrderFilterParameterType.sql`
|
|
- `034_CreateItemNumberFilterParameterType.sql`
|
|
- `035_CreateProfitCenterFilterParameterType.sql`
|
|
- `036_CreateWorkCenterFilterParameterType.sql`
|
|
- `037_CreateOperatorFilterParameterType.sql`
|
|
- `038_CreateComponentLotFilterParameterType.sql`
|
|
- `039_CreateItemOperationMisFilterParameterType.sql`
|
|
|
|
## C# Changes
|
|
|
|
### Files to Delete
|
|
|
|
```
|
|
src/JdeScoping.DataAccess/
|
|
├── Extensions/
|
|
│ └── TableValuedParameterExtensions.cs
|
|
├── Models/FilterEntries/
|
|
│ ├── WorkOrderFilterEntry.cs
|
|
│ ├── ItemNumberFilterEntry.cs
|
|
│ ├── ProfitCenterFilterEntry.cs
|
|
│ ├── WorkCenterFilterEntry.cs
|
|
│ ├── OperatorFilterEntry.cs
|
|
│ ├── ComponentLotFilterEntry.cs
|
|
│ └── ItemOperationMisFilterEntry.cs
|
|
```
|
|
|
|
### Files to Modify
|
|
|
|
**`ISearchQueryBuilder.cs`** - New signature:
|
|
```csharp
|
|
SearchQueryResult BuildSearchQuery(int searchId);
|
|
SearchQueryResult BuildMisQuery(int searchId);
|
|
SearchQueryResult BuildMisNonMatchQuery(int searchId);
|
|
```
|
|
|
|
**`SqlKataSearchQueryBuilder.cs`** - Generate SQL using functions:
|
|
```sql
|
|
-- Instead of TVP temp table population:
|
|
INSERT INTO #P_WorkOrders SELECT * FROM dbo.fn_GetSearchWorkOrders(@SearchId)
|
|
```
|
|
|
|
**`SearchModel.cs`** - Simplify:
|
|
- Remove all `List<*FilterEntry>` properties
|
|
- Remove all `*FilterEnabled` computed properties
|
|
- Keep: `Id`, `UserName`, `Name`, timestamps, results
|
|
|
|
**`SearchProcessor.cs`** - Pass `searchId` instead of filter lists
|
|
|
|
## Test Structure
|
|
|
|
```
|
|
tests/JdeScoping.Database.Tests/
|
|
├── Infrastructure/
|
|
│ └── DatabaseTestBase.cs
|
|
├── Functions/
|
|
│ ├── ScalarFunctionTests.cs
|
|
│ ├── SimpleTableFunctionTests.cs
|
|
│ └── ComplexTableFunctionTests.cs
|
|
└── Procedures/
|
|
└── ValidateSearchCriteriaProcedureTests.cs
|
|
```
|
|
|
|
### Test Categories
|
|
|
|
**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
|
|
|
|
Share with `Api.IntegrationTests` via `TestWebApplicationFactory` pattern.
|
|
|
|
## Documentation Updates
|
|
|
|
### OpenSpec Specifications
|
|
- Update `openspec/specs/data-access/spec.md` - Remove TVP references, add function requirements
|
|
- Update `openspec/specs/search-processing/spec.md` - Update query generation
|
|
- Update `openspec/specs/database-schema/spec.md` - Document extraction functions
|
|
|
|
### DOCUMENTATION Folder
|
|
- Update architecture diagrams
|
|
- Add `DOCUMENTATION/Database/ExtractionFunctions.md`
|
|
- Update testing documentation
|
|
|
|
## Implementation Order
|
|
|
|
### Phase 1: SQL Functions & Procedure
|
|
1. Create `045_CreateScalarExtractionFunctions.sql` (3 scalar functions)
|
|
2. Create `046_CreateSimpleTableFunctions.sql` (5 inline TVFs with CTE pattern)
|
|
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) - keep gaps, don't renumber
|
|
|
|
### Phase 2: Database Tests
|
|
6. Set up `DatabaseTestBase.cs` with xUnit Collection for isolation
|
|
7. Write `ScalarFunctionTests.cs`
|
|
8. Write `SimpleTableFunctionTests.cs`
|
|
9. Write `ComplexTableFunctionTests.cs`
|
|
10. Write `ValidateSearchCriteriaProcedureTests.cs`
|
|
11. Verify all tests pass
|
|
|
|
### Phase 3: C# Refactor
|
|
12. Update `ISearchQueryBuilder` interface
|
|
13. Update `SqlKataSearchQueryBuilder`
|
|
14. Update `SearchProcessor`
|
|
15. Simplify `SearchModel`
|
|
16. Delete `TableValuedParameterExtensions.cs`
|
|
17. Delete `FilterEntries/*.cs`
|
|
18. Update/delete related tests
|
|
|
|
### Phase 4: Integration & Verification
|
|
19. Run full test suite
|
|
20. Fix broken tests
|
|
21. Manual end-to-end verification
|
|
|
|
### Phase 5: Documentation
|
|
22. Update OpenSpec specifications
|
|
23. Update architecture documentation
|
|
24. Add Database.Tests documentation
|
|
25. Create ExtractionFunctions.md reference
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] 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 (functions + procedure)
|
|
- [ ] Existing integration tests passing
|
|
- [ ] Documentation updated
|