MemberJunction / MemberJunction/MJ

CodeGen string-quotes a column's DEFAULT expression, so spCreate<Entity> fails to compile and is left dropped (logged as a grey warning; CRUD validation still reports pass)

Open
#4,000 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
1d 8h
Merged PRs (30d)
308

Description

## Summary

CodeGen emits a column's `DEFAULT` into the generated create procedure **as a quoted string literal**. For a default that is an *expression* rather than a literal, the emitted SQL has nested single quotes, fails to compile, and the create procedure is **left dropped**. The entity can no longer be created at all, and the only signal is a grey warning line in the CodeGen log.

## What we saw

A perfectly ordinary schema change — giving a NOT NULL, server-assigned column a default:

```sql
ALTER TABLE [__mj_BizAppsContracts].[Contract]
ADD CONSTRAINT [DF_Contract_ContractNumber]
DEFAULT (N'CTR-' + FORMAT(NEXT VALUE FOR [__mj_BizAppsContracts].[seq_ContractNumber], N'D6'))
FOR [ContractNumber];
```

`sys.default_constraints.definition` then reads:

```
(N'CTR-'+format(NEXT VALUE FOR [__mj_BizAppsContracts].[seq_ContractNumber],N'D6'))
```

CodeGen generated `spCreateContract.sp.generated.sql` containing:

```sql
ISNULL(@ContractNumber, 'N'CTR-'+format(NEXT VALUE FOR [__mj_BizAppsContracts].[seq_ContractNumber],N'D6')'),
```

i.e. the whole expression wrapped in `'...'`, so the embedded `N'CTR-'` closes the literal immediately.

CodeGen output:

```
[CodeGen] SQL batch warning in _temp_batch_execution_...___mj_BizAppsContracts.sql: Incorrect syntax near '-'.
[CodeGen] SQL batch warning ...: Cannot find the object 'spCreateContract', because it does not exist or you do not have permission.
✖ Failed to apply permissions
✖ Error managing SQL scripts and execution
```

Afterwards `OBJECT_ID('__mj_BizAppsContracts.spCreateContract')` is **NULL** — the old proc was dropped and the new one never created. Every insert through the entity fails.

## Repro

1. Add a `DEFAULT` whose definition is an expression containing a string literal (the quotes are the trigger) to any column on any entity.
2. Run `mj codegen` (reproduced with `--skipfiles`; it is the SQL-generation path, not the file path).
3. `spCreate` is missing. `spUpdate`/`spDelete` and the other 18 procs in the schema survive.

Environment: MJ `6.1.0-edge.2`, `@memberjunction/cli/6.1.0-edge.2`, SQL Server 2022 (Linux container), schema `__mj_BizAppsContracts`.

## Two separate defects here

1. **The default is not treated as SQL.** A column default is an *expression*, not a value. It should be emitted verbatim (it already arrives fully parenthesised from `sys.default_constraints.definition`), not string-quoted. Simple literal defaults like `((0))` and `(newsequentialid())` survive today only because quoting them happens to still parse — `((0))` inside quotes is a harmless string, and it is silently the WRONG semantics even when it compiles.
2. **A failed proc creation is not treated as fatal.** The batch error is logged at warning level in grey, generation continues, `Post-CodeGen CRUD validation passed (486 entities checked)` still prints — and the run leaves the database with a missing create procedure. A dropped-and-not-recreated CRUD routine is a hard failure and should stop the run non-zero, or at minimum the CRUD validation should catch precisely this.

Point 2 is the one that made this expensive: the log said "passed" while contract creation was broken.

## Suggested fix

- Emit the default expression unquoted in the generated `ISNULL(@Param, )`. If a literal-vs-expression distinction is wanted, `sys.default_constraints` already parenthesises the definition; the safe default is to treat it as SQL.
- Fail the run when a generated CRUD routine does not exist after execution, and have `Post-CodeGen CRUD validation` assert `spCreate*` presence (it evidently does not, or it passed here in error).

## Workaround

Do not give a column an expression default if MJ generates its CRUD. We reverted ours in a forward-only migration and re-ran CodeGen, which restored `spCreateContract`.

Contributor guide

Open the contributing guide

Research direction

Start with the SQL-generation path exercised by `mj codegen --skipfiles`, tracing `sys.default_constraints.definition` into the generated `spCreate` procedure. Then inspect the Post-CodeGen CRUD validation, especially its handling of missing create procedures. Done means expression defaults are emitted as SQL, failed procedure creation is surfaced as a failure, and validation detects missing `spCreate*` routines.

Written by the indexing model from the issue text.

Assessment

Tech stack
sql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.