docs: update design with inline TVFs based on Codex review
Key changes from Codex feedback: - THROW cannot be used in UDFs (SQL Server restriction) - Multi-statement TVFs have poor performance (no stats) - Changed to inline TVFs for extraction (optimal performance) - Added validation stored procedure for strict error handling - Use TRY_CONVERT for safe type conversion - Use OPENJSON...WITH for complex object extraction
This commit is contained in:
@@ -16,7 +16,19 @@ Create SQL Server functions to extract values from the `Search.Criteria` JSON co
|
|||||||
|
|
||||||
### SQL Server Version
|
### SQL Server Version
|
||||||
- **Target:** SQL Server 2022
|
- **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
|
### 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_GetSearchMaximumDt` | `DATETIME2(7)` | `$.MaximumDt` |
|
||||||
| `dbo.fn_GetSearchExtractMisData` | `BIT` | `$.ExtractMisData` |
|
| `dbo.fn_GetSearchExtractMisData` | `BIT` | `$.ExtractMisData` |
|
||||||
|
|
||||||
**Simple Table Functions (5):**
|
**Simple Table Functions (5) - Inline TVFs:**
|
||||||
| Function | Returns | JSON Path |
|
| Function | Returns | JSON Path |
|
||||||
|----------|---------|-----------|
|
|----------|---------|-----------|
|
||||||
| `dbo.fn_GetSearchWorkOrders` | `TABLE(WorkOrderNumber BIGINT)` | `$.WorkOrderNumbers` |
|
| `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_GetSearchWorkCenters` | `TABLE(Code VARCHAR(12))` | `$.WorkCenters` |
|
||||||
| `dbo.fn_GetSearchOperatorIDs` | `TABLE(OperatorID VARCHAR(128))` | `$.OperatorIDs` |
|
| `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 |
|
| Function | Returns | JSON Path |
|
||||||
|----------|---------|-----------|
|
|----------|---------|-----------|
|
||||||
| `dbo.fn_GetSearchComponentLots` | `TABLE(LotNumber VARCHAR(30), ItemNumber VARCHAR(128))` | `$.ComponentLotNumbers[*]` |
|
| `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[*]` |
|
| `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 |
|
| 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` |
|
| 50002 | Criteria is NULL or empty | `Search ID {id} has no criteria` |
|
||||||
| 50003 | Criteria is invalid JSON | `Search ID {id} has invalid JSON` |
|
| 50003 | Criteria is invalid JSON | `Search ID {id} has invalid JSON` |
|
||||||
|
|
||||||
**Normal empty results (no error):**
|
### Inline TVF Pattern (Simple Arrays)
|
||||||
- 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:
|
|
||||||
|
|
||||||
```sql
|
```sql
|
||||||
CREATE FUNCTION dbo.fn_GetSearchWorkOrders(@SearchId INT)
|
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
|
AS
|
||||||
BEGIN
|
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 @Criteria VARCHAR(MAX);
|
||||||
DECLARE @ErrorMsg NVARCHAR(400);
|
DECLARE @ErrorMsg NVARCHAR(400);
|
||||||
|
|
||||||
-- Get criteria
|
|
||||||
SELECT @Criteria = Criteria
|
SELECT @Criteria = Criteria
|
||||||
FROM dbo.Search
|
FROM dbo.Search
|
||||||
WHERE ID = @SearchId;
|
WHERE ID = @SearchId;
|
||||||
|
|
||||||
-- Validate search exists
|
|
||||||
IF @@ROWCOUNT = 0
|
IF @@ROWCOUNT = 0
|
||||||
BEGIN
|
BEGIN
|
||||||
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' not found');
|
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' not found');
|
||||||
THROW 50001, @ErrorMsg, 1;
|
THROW 50001, @ErrorMsg, 1;
|
||||||
END
|
END
|
||||||
|
|
||||||
-- Validate criteria not null/empty
|
|
||||||
IF @Criteria IS NULL OR @Criteria = ''
|
IF @Criteria IS NULL OR @Criteria = ''
|
||||||
BEGIN
|
BEGIN
|
||||||
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has no criteria');
|
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has no criteria');
|
||||||
THROW 50002, @ErrorMsg, 1;
|
THROW 50002, @ErrorMsg, 1;
|
||||||
END
|
END
|
||||||
|
|
||||||
-- Validate JSON
|
|
||||||
IF ISJSON(@Criteria) = 0
|
IF ISJSON(@Criteria) = 0
|
||||||
BEGIN
|
BEGIN
|
||||||
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has invalid JSON');
|
SET @ErrorMsg = CONCAT('Search ID ', @SearchId, ' has invalid JSON');
|
||||||
THROW 50003, @ErrorMsg, 1;
|
THROW 50003, @ErrorMsg, 1;
|
||||||
END
|
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
|
END
|
||||||
|
GO
|
||||||
```
|
```
|
||||||
|
|
||||||
## Migration Scripts
|
## Migration Scripts
|
||||||
@@ -108,21 +181,24 @@ END
|
|||||||
### New Scripts
|
### New Scripts
|
||||||
|
|
||||||
**045_CreateScalarExtractionFunctions.sql**
|
**045_CreateScalarExtractionFunctions.sql**
|
||||||
- `fn_GetSearchMinimumDt`
|
- `fn_GetSearchMinimumDt` (scalar)
|
||||||
- `fn_GetSearchMaximumDt`
|
- `fn_GetSearchMaximumDt` (scalar)
|
||||||
- `fn_GetSearchExtractMisData`
|
- `fn_GetSearchExtractMisData` (scalar)
|
||||||
|
|
||||||
**046_CreateSimpleTableFunctions.sql**
|
**046_CreateSimpleTableFunctions.sql** (inline TVFs)
|
||||||
- `fn_GetSearchWorkOrders`
|
- `fn_GetSearchWorkOrders`
|
||||||
- `fn_GetSearchItemNumbers`
|
- `fn_GetSearchItemNumbers`
|
||||||
- `fn_GetSearchProfitCenters`
|
- `fn_GetSearchProfitCenters`
|
||||||
- `fn_GetSearchWorkCenters`
|
- `fn_GetSearchWorkCenters`
|
||||||
- `fn_GetSearchOperatorIDs`
|
- `fn_GetSearchOperatorIDs`
|
||||||
|
|
||||||
**047_CreateComplexTableFunctions.sql**
|
**047_CreateComplexTableFunctions.sql** (inline TVFs with OPENJSON...WITH)
|
||||||
- `fn_GetSearchComponentLots`
|
- `fn_GetSearchComponentLots`
|
||||||
- `fn_GetSearchPartOperations`
|
- `fn_GetSearchPartOperations`
|
||||||
|
|
||||||
|
**048_CreateValidateSearchCriteriaProcedure.sql**
|
||||||
|
- `usp_ValidateSearchCriteria` (stored procedure with THROW)
|
||||||
|
|
||||||
### Scripts to Delete
|
### Scripts to Delete
|
||||||
|
|
||||||
Remove obsolete Table Type scripts:
|
Remove obsolete Table Type scripts:
|
||||||
@@ -184,13 +260,34 @@ tests/JdeScoping.Database.Tests/
|
|||||||
│ ├── ScalarFunctionTests.cs
|
│ ├── ScalarFunctionTests.cs
|
||||||
│ ├── SimpleTableFunctionTests.cs
|
│ ├── SimpleTableFunctionTests.cs
|
||||||
│ └── ComplexTableFunctionTests.cs
|
│ └── ComplexTableFunctionTests.cs
|
||||||
|
└── Procedures/
|
||||||
|
└── ValidateSearchCriteriaProcedureTests.cs
|
||||||
```
|
```
|
||||||
|
|
||||||
### Test Categories
|
### Test Categories
|
||||||
|
|
||||||
**Happy Path:** Valid JSON, correct extraction, empty arrays
|
**Inline TVF Tests (graceful behavior):**
|
||||||
**Error Tests:** Non-existent SearchId, NULL criteria, invalid JSON
|
- Valid JSON → correct extraction
|
||||||
**Edge Cases:** Large arrays, special characters, NULL values in arrays, Unicode
|
- 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
|
### Test Infrastructure
|
||||||
|
|
||||||
@@ -210,48 +307,51 @@ Share with `Api.IntegrationTests` via `TestWebApplicationFactory` pattern.
|
|||||||
|
|
||||||
## Implementation Order
|
## Implementation Order
|
||||||
|
|
||||||
### Phase 1: SQL Functions
|
### Phase 1: SQL Functions & Procedure
|
||||||
1. Create `045_CreateScalarExtractionFunctions.sql`
|
1. Create `045_CreateScalarExtractionFunctions.sql` (3 scalar functions)
|
||||||
2. Create `046_CreateSimpleTableFunctions.sql`
|
2. Create `046_CreateSimpleTableFunctions.sql` (5 inline TVFs)
|
||||||
3. Create `047_CreateComplexTableFunctions.sql`
|
3. Create `047_CreateComplexTableFunctions.sql` (2 inline TVFs with OPENJSON...WITH)
|
||||||
4. Delete obsolete Table Type scripts (033-039)
|
4. Create `048_CreateValidateSearchCriteriaProcedure.sql` (validation procedure)
|
||||||
5. Renumber remaining scripts (040-044 → 033-037)
|
5. Delete obsolete Table Type scripts (033-039)
|
||||||
|
6. Renumber remaining scripts (040-044 → 033-037)
|
||||||
|
|
||||||
### Phase 2: Database Tests
|
### Phase 2: Database Tests
|
||||||
6. Set up `DatabaseTestBase.cs`
|
7. Set up `DatabaseTestBase.cs`
|
||||||
7. Write `ScalarFunctionTests.cs`
|
8. Write `ScalarFunctionTests.cs`
|
||||||
8. Write `SimpleTableFunctionTests.cs`
|
9. Write `SimpleTableFunctionTests.cs`
|
||||||
9. Write `ComplexTableFunctionTests.cs`
|
10. Write `ComplexTableFunctionTests.cs`
|
||||||
10. Verify all tests pass
|
11. Write `ValidateSearchCriteriaProcedureTests.cs`
|
||||||
|
12. Verify all tests pass
|
||||||
|
|
||||||
### Phase 3: C# Refactor
|
### Phase 3: C# Refactor
|
||||||
11. Update `ISearchQueryBuilder` interface
|
13. Update `ISearchQueryBuilder` interface
|
||||||
12. Update `SqlKataSearchQueryBuilder`
|
14. Update `SqlKataSearchQueryBuilder`
|
||||||
13. Update `SearchProcessor`
|
15. Update `SearchProcessor`
|
||||||
14. Simplify `SearchModel`
|
16. Simplify `SearchModel`
|
||||||
15. Delete `TableValuedParameterExtensions.cs`
|
17. Delete `TableValuedParameterExtensions.cs`
|
||||||
16. Delete `FilterEntries/*.cs`
|
18. Delete `FilterEntries/*.cs`
|
||||||
17. Update/delete related tests
|
19. Update/delete related tests
|
||||||
|
|
||||||
### Phase 4: Integration & Verification
|
### Phase 4: Integration & Verification
|
||||||
18. Run full test suite
|
20. Run full test suite
|
||||||
19. Fix broken tests
|
21. Fix broken tests
|
||||||
20. Manual end-to-end verification
|
22. Manual end-to-end verification
|
||||||
|
|
||||||
### Phase 5: Documentation
|
### Phase 5: Documentation
|
||||||
21. Update OpenSpec specifications
|
23. Update OpenSpec specifications
|
||||||
22. Update architecture documentation
|
24. Update architecture documentation
|
||||||
23. Add Database.Tests documentation
|
25. Add Database.Tests documentation
|
||||||
24. Create ExtractionFunctions.md reference
|
26. Create ExtractionFunctions.md reference
|
||||||
|
|
||||||
## Acceptance Criteria
|
## Acceptance Criteria
|
||||||
|
|
||||||
- [ ] All 11 SQL functions created and working
|
- [ ] All 11 SQL inline TVFs/scalar functions created and working
|
||||||
- [ ] Error codes 50001/50002/50003 thrown for invalid inputs
|
- [ ] Validation stored procedure created with THROW for errors
|
||||||
- [ ] Empty results returned for missing/null/empty properties
|
- [ ] Inline TVFs return empty results for all edge cases (graceful)
|
||||||
- [ ] Table Type scripts removed
|
- [ ] Validation procedure throws 50001/50002/50003 for invalid inputs (strict)
|
||||||
- [ ] C# TVP code removed
|
- [ ] Table Type scripts (033-039) removed
|
||||||
|
- [ ] C# TVP code removed (TableValuedParameterExtensions, FilterEntries)
|
||||||
- [ ] Query builder uses SearchId parameter only
|
- [ ] Query builder uses SearchId parameter only
|
||||||
- [ ] Database.Tests passing (all categories)
|
- [ ] Database.Tests passing (functions + procedure)
|
||||||
- [ ] Existing integration tests passing
|
- [ ] Existing integration tests passing
|
||||||
- [ ] Documentation updated
|
- [ ] Documentation updated
|
||||||
|
|||||||
Reference in New Issue
Block a user