fixed RemoveProvisionAsync not removing triggers

[gtrafford]

RemoveProvisionAsync in the SqliteProvider was not actually removing the triggers created by ApplyProvisionAsync. For some reason it was creating a list of column names and adding them to a list called listOfTables, and then looping through this to look for the triggers, which were just created based on table names, not columns.

[adospace]

Thanks @gtrafford — the change is correct, and I dug through the history to answer the "for some reason" in your description. It turns out listOfTables was never intentional: it's the residue of a copy-paste that a refactor broke one day later.

Where it came from

5330ab3 (2020-02-12, "Added RemoveProvisionAsync implementation") wrote RemoveProvisionAsync by pasting the column-discovery block out of InitializeStoreAsync. That block ran PRAGMA table_info per configured table and filled table.Columns; step 5 then dropped triggers for:

foreach (var table in Configuration.Tables.Cast<SqliteSyncTable>().Where(_ => _.Columns.Any()))

So the PRAGMA loop's only surviving role in the teardown was an existence probe — PRAGMA table_info on a table that isn't in the database returns zero rows, so Columns stays empty and the table is skipped. It was already redundant even then, since the teardown uses DROP TRIGGER IF EXISTS.

The __OP check is vestigial in the same way. In the code it was pasted from, it threw NotSupportedException — that's a provisioning-time validation, because __OP is the alias CoreSync reserves for the change-op column (CT.OP AS __OP in SqliteSyncTable.IncrementalAddOrUpdatesQuery). It still lives in its rightful place in InitializeStoreAsync. It means nothing when tearing down.

Where it broke

8cb14f1 (2020-02-13, "Improving perfomance of change table"), the very next day. SqliteSyncTable.Columns changed from a List to a Dictionary, so table.Columns.Add(new SqliteColumn(...)) stopped compiling. Instead of deleting the now-pointless loop, the commit just made it compile again:

-        table.Columns.Add(new SqliteColumn(colName, colType, pk));
+        listOfTables.Add(colName);
...
-foreach (var table in Configuration.Tables.Cast<SqliteSyncTable>().Where(_ => _.Columns.Any()))
+foreach (var tableName in listOfTables)

Column names into a variable named listOfTables, and the throw softened to a continue. Triggers are named __{table.Name}_ct-{op}__, so from that commit onward the loop dropped nothing at all — unless a column happened to share a table's name.

Why the fix is right

• Iterating table names is the only thing that can ever match the trigger names produced by SetupTableForFullChangeDetection / SetupTableForUpdatesOrDeletesOnly.
• Dropping the Where(_ => _.Columns.Any()) guard is safe: DROP TRIGGER IF EXISTS already handles a table that isn't there.
• It brings SQLite in line with every other provider — MySqlSyncProvider, PostgreSQLSyncProvider and SqlSyncProvider all already loop Configuration.Tables and call DisableChangeTrackingForTable(cmd, table.Name, ct). SQLite was the odd one out.

Worth spelling out how bad this was: __CORE_SYNC_CT is dropped, and the orphaned triggers still insert into it, so after RemoveProvisionAsync every tracked table became unwritable with no such table: main.__CORE_SYNC_CT.

Tests

I pushed src/CoreSync.Tests/SqliteRemoveProvisionTests.cs to this branch — self-contained, temp-file SQLite, no external server:

test	before	after
RemovesChangeTrackingTriggersAndTables	❌ 6 triggers left behind	✅
LeavesDatabaseWritable	❌ no such table: main.__CORE_SYNC_CT	✅
CanBeCalledOnANotProvisionedDatabase	✅	✅
RemovesTriggersOfATableMissingFromTheDatabase	❌ 3 triggers left behind	✅

The last one is exactly the case the old Columns.Any() guard was meant to cover, and it confirms removing the guard is safe.

One optional follow-up

The method drops __CORE_SYNC_CT first and the triggers last. If a trigger drop ever throws halfway through, the database is left in precisely the bricked state this PR fixes. Moving the trigger loop ahead of the three DROP TABLEs would be strictly more robust (SqlServerCTProvider already disables tracking first). The tests pass either way, so this is fine as a separate commit — not a blocker for merging.