Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
717 changes: 717 additions & 0 deletions docs/cli/imports.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Change: Fix apply dry-run false-change detection

## Why
After running `import --force` followed by `apply --dry-run`, the system incorrectly
reports relation creates/deletes, policy table reassignments, and RPC updates even
though no code was modified. This makes the dry-run output noisy and untrustworthy.

## What Changes
- Fix relation comparison to skip duplicate FK constraints for already-matched columns
- Fix relation comparison to skip cross-schema FK references (e.g., `auth.users`)
- Fix relation index creation check to only fire when target has index but source doesn't
- Fix policy comparison to match by schema+table+name instead of name only
- Fix RPC state extraction to preserve stored CompleteStatement from previous import

## Impact
- Affected specs: cli-imports
- Affected code:
- `pkg/resource/tables/compare.go` (relation comparison)
- `pkg/resource/policies/compare.go` (policy comparison)
- `pkg/state/rpc.go` (RPC state extraction)
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## MODIFIED Requirements

### Requirement: Relation Comparison Accuracy
The relation comparison SHALL correctly match source and target relations even when the
database contains duplicate FK constraints for the same column or cross-schema FK references.

#### Scenario: Duplicate FK constraints on same column
- **WHEN** the remote database has two FK constraints for the same source column (e.g., custom-named `fk_mc_division` and default-named `master_creators_division_id_fkey`)
- **AND** the local code has one FK for that column
- **THEN** the matched constraint SHALL be recognized as identical
- **AND** the duplicate constraint SHALL NOT be flagged as a delete

#### Scenario: Cross-schema FK reference
- **WHEN** the remote database has a FK referencing a table in a different schema (e.g., `public.user_brands.user_id → auth.users.id`)
- **AND** the local code does not represent this FK (because the target table is not in the imported model set)
- **THEN** the cross-schema FK SHALL NOT be flagged as a delete

#### Scenario: Index creation check
- **WHEN** both local and remote sides have no index for a relation
- **THEN** no index creation SHALL be proposed
- **WHEN** the remote has an index but the local does not
- **THEN** an index creation item SHALL be proposed

### Requirement: Policy Comparison Accuracy
The policy comparison SHALL match policies by their full identity (schema, table, and name)
rather than by name alone, to prevent cross-table mismatches when multiple tables share
the same policy name.

#### Scenario: Same-named policies on different tables
- **WHEN** multiple tables have policies with the same name (e.g., "admin full access" on `products` and `product_interaction_performance`)
- **THEN** each policy SHALL be compared only with its corresponding policy on the same table
- **AND** no false table-change diffs SHALL be reported

### Requirement: RPC Comparison Accuracy
The RPC state extraction SHALL preserve the CompleteStatement from the previous import
rather than rebuilding it from the Go struct template, to avoid format-only differences
between `BuildRpc()` output and `pg_get_functiondef()` output.

#### Scenario: No code changes after import
- **WHEN** a user runs import and then apply without modifying any RPC code
- **THEN** no RPC updates SHALL be detected
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## 1. Investigation
- [x] 1.1 Trace relation false creates/deletes — root cause: duplicate FK constraints + cross-schema FKs
- [x] 1.2 Trace policy false table reassignment — root cause: CompareList maps by name only, not schema+table+name
- [x] 1.3 Trace RPC false updates — root cause: BindRpcFunction overwrites state CompleteStatement with BuildRpc() output

## 2. Implementation
- [x] 2.1 Add `matchedSourceCols` tracking to skip duplicate FK deletes in `compareRelations`
- [x] 2.2 Add cross-schema FK filter to skip FKs where TargetTableSchema != SourceSchema
- [x] 2.3 Change index creation condition from `t.Index == nil && sc.Index == nil` to `t.Index != nil && sc.Index == nil`
- [x] 2.4 Fix policy `CompareList` to use schema+table+name key instead of name only
- [x] 2.5 Preserve state CompleteStatement in `ExtractRpc` for existing functions

## 3. Validation
- [x] 3.1 All existing tests pass
- [x] 3.2 Pivot project: `import` shows zero conflicts
- [x] 3.3 Pivot project: `apply --dry-run` shows only genuinely new resources
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Change: Fix model relation TargetTableName derivation

## Why
`addModelRelation` in `pkg/state/table.go` derives `TargetTableName` from the struct **field name** (`utils.ToSnakeCase(field.Name)`) instead of the referenced **type name**. When a field name differs from the type name (e.g., field `MasterCreatorBrand` of type `*MasterCreators`), the relation points to a non-existent table (`master_creator_brand` instead of `master_creators`). This causes `validateTableRelations` during `apply` to fail with "target column id is not exist in table master_creator_brand".

## What Changes
- Fix `addModelRelation` in `pkg/state/table.go` to resolve `TargetTableName` from the field's type name using `findTypeName()`, matching the behavior of `addStateRelation`

## Impact
- Affected specs: cli-imports
- Affected code: `pkg/state/table.go`
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
## MODIFIED Requirements

### Requirement: Comparison and Diff Checks

The system SHALL compare remote (Supabase) resources against existing local resources to detect drift, unless `--force` is set. Pointer-typed fields (e.g., `*string`) SHALL be compared by dereferenced value, not by pointer address. Slice fields SHALL be compared by element values, not by slice indices. Relation action comparisons SHALL only flag a conflict when action data is available from both sides, or when running in apply mode. Relation matching SHALL fall back to `schema.table.column` lookup when constraint name lookup fails. Cross-schema FK references SHALL be filtered out before comparison when the target table is not in the local model set. RPC `CompleteStatement` comparison during import SHALL use the stored state value, not the rebuilt value from `BuildRpc()`. Model relation `TargetTableName` SHALL be derived from the referenced type name, not from the struct field name, to ensure it matches the actual database table.

#### Scenario: Normal comparison
- **WHEN** `--force` is not set and existing resources are present
- **THEN** the system SHALL run comparison checks for types, tables, roles, RPC functions, and storages

#### Scenario: Comparison error in normal mode
- **WHEN** a comparison detects conflicting changes
- **THEN** the system SHALL return an error and abort the import

#### Scenario: Comparison error in dry-run mode
- **WHEN** `--dry-run` is set and a comparison detects conflicts
- **THEN** the system SHALL collect the error message without aborting, and report it at the end

#### Scenario: Skip comparisons with force flag
- **WHEN** `--force` is set
- **THEN** all comparison checks SHALL be skipped and remote state SHALL overwrite local unconditionally

#### Scenario: Skip comparison when no existing resources
- **WHEN** the Existing set for a resource type is empty (first import or new resource type)
- **THEN** the comparison for that resource type SHALL be skipped

#### Scenario: No false conflict on identical pointer-typed fields
- **WHEN** a type's `Comment` field has the same string value on both remote and local but different pointer addresses
- **THEN** the comparison SHALL report no conflict

#### Scenario: No false conflict on identical enum values
- **WHEN** a type's `Enums` slice has the same string values on both remote and local
- **THEN** the comparison SHALL report no conflict regardless of slice allocation

#### Scenario: No false conflict on identical attribute values
- **WHEN** a type's `Attributes` slice has the same `Name` and `TypeID` values on both remote and local
- **THEN** the comparison SHALL report no conflict regardless of slice allocation

#### Scenario: No false conflict on missing remote relation action during import
- **WHEN** a relation's remote `Action` is nil (not attached) but local `Action` is populated, and the comparison mode is import
- **THEN** the comparison SHALL NOT flag this as a conflict

#### Scenario: Flag missing action as conflict in apply mode
- **WHEN** a relation's local `Action` is populated but remote `Action` is nil, and the comparison mode is apply
- **THEN** the comparison SHALL flag this as a conflict for `OnUpdate` and `OnDelete` actions

#### Scenario: No false conflict on constraint name mismatch
- **WHEN** a relation exists in both remote and local with different constraint names but identical `schema.table.column` reference
- **THEN** the comparison SHALL match them via fallback lookup and report no conflict

#### Scenario: No false conflict on cross-schema FK references
- **WHEN** a remote table has a FK referencing a table in a different schema (e.g., `auth.users`) that is not in the local model set
- **THEN** the comparison SHALL exclude that relationship before comparison

#### Scenario: No false conflict on RPC CompleteStatement formatting
- **WHEN** an RPC function's `CompleteStatement` from `pg_get_functiondef()` differs from the `BuildRpc()` rebuilt version only in formatting (param prefix, default quoting, search_path)
- **THEN** the import comparison SHALL use the stored state `CompleteStatement` and report no conflict

#### Scenario: Correct TargetTableName for model relations
- **WHEN** a model struct field references a type with a different name than the field (e.g., field `MasterCreatorBrand` of type `*MasterCreators`)
- **THEN** the relation `TargetTableName` SHALL be derived from the type name (`master_creators`), not the field name (`master_creator_brand`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## 1. Implementation
- [x] 1.1 Fix `addModelRelation` to use `raiden.GetTableName()` on the field's type for `TargetTableName` instead of `field.Name`

## 2. Testing
- [x] 2.1 Run state tests (`go test ./pkg/state/`)
- [x] 2.2 Run resource tests (`go test ./pkg/resource/...`)
- [x] 2.3 Validate in pivot project with `apply --dry-run`
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Change: Fix false-positive conflicts during import comparison

## Why
The import comparison logic produces false-positive conflicts that block `raiden imports` after a clean `--force` import across all three resource comparison subsystems:

1. **Type comparison**: Compares `*string` pointer addresses instead of values for `Comment` fields; iterates slice indices instead of values for `Enums` and `Attributes`.
2. **Table relation comparison**: Constraint name mismatch between remote (real DB names like `fk_mc_division`) and local (generated names like `public_table_col_fkey`) causes relations to be flagged as new. Missing remote `Action` data treated as conflict during import. Cross-schema FK references (e.g., `auth.users`) not filtered before comparison.
3. **RPC function comparison**: `BindRpcFunction` overwrites state's stored `CompleteStatement` with rebuilt one from `BuildRpc()`, which differs from `pg_get_functiondef()` in parameter prefix (`in_`), default value quoting (`'null'::uuid` vs `null::uuid`), and `search_path` inclusion.

## What Changes
### Type comparison (`pkg/resource/types/compare.go`)
- Fix `Comment` pointer comparison to dereference and compare string values
- Remove duplicate `Comment` comparison block
- Fix `Enums` and `Attributes` range loops to iterate values instead of indices

### Table relation comparison (`pkg/resource/tables/compare.go`)
- Add secondary index `mapTargetByCol` keyed by `schema.table.column` for fallback relation matching when constraint name lookup fails
- Guard relation `Action` nil check to only flag as conflict in apply mode, not import mode

### Table state extraction (`pkg/state/table.go`)
- Add fallback relation lookup by `SourceTableName+SourceColumnName` in `buildTableRelation` when constraint name lookup fails

### Cross-schema relation filtering (`pkg/resource/import.go`)
- Filter remote relationships to exclude references to tables not in the local model set before comparison, mirroring generator behavior that skips cross-schema FKs

### RPC comparison (`pkg/resource/import.go`)
- Restore state's stored `CompleteStatement` before comparison instead of using rebuilt one from `BuildRpc()`, so import only detects real remote changes

## Impact
- Affected specs: cli-imports
- Affected code: `pkg/resource/types/compare.go`, `pkg/resource/tables/compare.go`, `pkg/state/table.go`, `pkg/resource/import.go`
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
## MODIFIED Requirements

### Requirement: Comparison and Diff Checks

The system SHALL compare remote (Supabase) resources against existing local resources to detect drift, unless `--force` is set. Pointer-typed fields (e.g., `*string`) SHALL be compared by dereferenced value, not by pointer address. Slice fields SHALL be compared by element values, not by slice indices. Relation action comparisons SHALL only flag a conflict when action data is available from both sides, or when running in apply mode. Relation matching SHALL fall back to `schema.table.column` lookup when constraint name lookup fails. Cross-schema FK references SHALL be filtered out before comparison when the target table is not in the local model set. RPC `CompleteStatement` comparison during import SHALL use the stored state value, not the rebuilt value from `BuildRpc()`.

#### Scenario: Normal comparison
- **WHEN** `--force` is not set and existing resources are present
- **THEN** the system SHALL run comparison checks for types, tables, roles, RPC functions, and storages

#### Scenario: Comparison error in normal mode
- **WHEN** a comparison detects conflicting changes
- **THEN** the system SHALL return an error and abort the import

#### Scenario: Comparison error in dry-run mode
- **WHEN** `--dry-run` is set and a comparison detects conflicts
- **THEN** the system SHALL collect the error message without aborting, and report it at the end

#### Scenario: Skip comparisons with force flag
- **WHEN** `--force` is set
- **THEN** all comparison checks SHALL be skipped and remote state SHALL overwrite local unconditionally

#### Scenario: Skip comparison when no existing resources
- **WHEN** the Existing set for a resource type is empty (first import or new resource type)
- **THEN** the comparison for that resource type SHALL be skipped

#### Scenario: No false conflict on identical pointer-typed fields
- **WHEN** a type's `Comment` field has the same string value on both remote and local but different pointer addresses
- **THEN** the comparison SHALL report no conflict

#### Scenario: No false conflict on identical enum values
- **WHEN** a type's `Enums` slice has the same string values on both remote and local
- **THEN** the comparison SHALL report no conflict regardless of slice allocation

#### Scenario: No false conflict on identical attribute values
- **WHEN** a type's `Attributes` slice has the same `Name` and `TypeID` values on both remote and local
- **THEN** the comparison SHALL report no conflict regardless of slice allocation

#### Scenario: No false conflict on missing remote relation action during import
- **WHEN** a relation's remote `Action` is nil (not attached) but local `Action` is populated, and the comparison mode is import
- **THEN** the comparison SHALL NOT flag this as a conflict

#### Scenario: Flag missing action as conflict in apply mode
- **WHEN** a relation's local `Action` is populated but remote `Action` is nil, and the comparison mode is apply
- **THEN** the comparison SHALL flag this as a conflict for `OnUpdate` and `OnDelete` actions

#### Scenario: No false conflict on constraint name mismatch
- **WHEN** a relation exists in both remote and local with different constraint names but identical `schema.table.column` reference
- **THEN** the comparison SHALL match them via fallback lookup and report no conflict

#### Scenario: No false conflict on cross-schema FK references
- **WHEN** a remote table has a FK referencing a table in a different schema (e.g., `auth.users`) that is not in the local model set
- **THEN** the comparison SHALL exclude that relationship before comparison

#### Scenario: No false conflict on RPC CompleteStatement formatting
- **WHEN** an RPC function's `CompleteStatement` from `pg_get_functiondef()` differs from the `BuildRpc()` rebuilt version only in formatting (param prefix, default quoting, search_path)
- **THEN** the import comparison SHALL use the stored state `CompleteStatement` and report no conflict
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## 1. Type comparison fixes
- [x] 1.1 Fix `Comment` pointer comparison in `CompareItem()` to dereference `*string` values
- [x] 1.2 Remove duplicate `Comment` comparison block (lines 106-115)
- [x] 1.3 Fix `Enums` range loop to iterate values (`for _, se := range`) instead of indices
- [x] 1.4 Fix `Attributes` range loop to iterate values and compare `Name`/`TypeID` fields

## 2. Table relation comparison fixes
- [x] 2.1 Add `mapTargetByCol` secondary index in `compareRelations()` for fallback matching by `schema.table.column`
- [x] 2.2 Guard relation `Action` nil check to only flag diff in apply mode (not import)
- [x] 2.3 Add fallback relation lookup in `buildTableRelation()` by `SourceTableName+SourceColumnName`
- [x] 2.4 Add cross-schema relation filtering in `compareTables()` to exclude FKs referencing tables not in local model set

## 3. RPC comparison fix
- [x] 3.1 Restore state `CompleteStatement` in `importJob.compareRpc()` before comparison instead of using rebuilt value from `BuildRpc()`

## 4. Testing
- [x] 4.1 Run existing type comparison tests (`go test ./pkg/resource/types/`)
- [x] 4.2 Run table comparison tests (`go test ./pkg/resource/tables/`)
- [x] 4.3 Run state tests (`go test ./pkg/state/`)
- [x] 4.4 Run import tests (`go test ./pkg/resource/`)
- [x] 4.5 Run RPC tests (`go test ./pkg/resource/rpc/`)
- [x] 4.6 Validate in pivot project — zero false-positive conflicts
Loading