include table name in DEFAULT-constraint names in MSSQL

[szro0]

Hi Levente,

Issue in MSSQL: the DEFAULT-constraint-name generated by pysqlsync is not table-specific which prevents the creation of multiple tables with the same column-name and those columns having default values.

Example:
Python dataclasses:

@dataclass
class Table1:
    id: PrimaryKey[int]
    boolean_field: bool = False

@dataclass
class Table2:
    id: PrimaryKey[int]
    boolean_field: bool = False

When creating DB tables for these classes in MSSQL, the column definition of the boolean column looks like this for both of the tables:

"boolean_field" bit NOT NULL CONSTRAINT "df_boolean_field" DEFAULT 0

The problem is that df_boolean_field is not unique, and will cause an error when attempting to create the second table in the database, like There is already an object named 'df_boolean_field' in the database..

In this PR I change the generated constraint name to include the table name as well. So the constraints in the previous example would be generated with these names: df_Table1_boolean_field and df_Table2_boolean_field, which are now unique.

To be able to include the table name in the constraint, I added the table_name field to the Column dataclass (see pysqlsync/formation/object_types.py), which can be utilized in MSSQLColumn (see pysqlsync/dialect/mssql/object_types.py). Let me know if this is not desired, or propose a better solution.

[hunyadi]

I am a little hesitant to add table_name to Column. Originally, I wanted to ensure a top-down hierarchy of dependencies, e.g. a Namespace knows what Table objects it holds but Table objects don't refer back to Namespace, Table objects keep track of Column objects they hold but a Column doesn't have a back-reference, etc. This has some favorable properties, e.g. when you drop, move or rename objects, you can avoid recursive updates to the object model. Today, the code doesn't have logic to check or update names when a parent object changes, specifically to update all Column objects when a Table is moved or renamed.

On the other hand, the use of default_constraint_name seems localized. It is defined on MSSQLColumn. One of the callers, mutate_table_stmt has access to a Table object whose name it could pass to default_constraint_name. Unfortunately, data_spec in Column is trickier, it's used both directly and via column_spec, in contexts where no Table object seems readily available. Likewise, drop_stmt has some contextual dependency. Let me look into this in more depth.

[hunyadi]

One idea I have in mind is to treat DEFAULT similar to other constraints, i.e. extend the implementation in add_constraints_stmt and drop_constraints_stmt, both of which have access to the MSSQLTable instance. When I look at mutate_table_stmt in MSSQLMutator, I already see this approach in place.

[szro0]

Thank you, I get your point, I'll check that approach.

[hunyadi]

I explored a third option, which is to add the constraint name as an attribute exclusive to the MSSQLColumn class.

Unfortunately, this approach has some ripple effects in terms of syntax, specifically how Column (sub-)classes are constructed; the class Column had attributes with default initial values, which would normally cause MSSQLColumn to have attributes with default initial values only, which forced me to introduce a factory function create on Column to keep the possibility of optional arguments but not enforce anything on derived classes. Furthermore, I had to modify database object discovery for Microsoft SQL Server to fetch the constraint name for a default column. The constraint name for default columns (intentionally) doesn't participate in testing object equality.

These changes are pushed to the branch dev as the approach may need some refinement. In particular, tests fail due to the random strings, we will need to set a fixed seed for reproducible tests (or use string equality assertions with masking).

[hunyadi]

I have implemented string comparison with masking enabled, allowing randomly generated character sequences to be ignored in unit and integration tests. Test execution now yields green on the branch dev.

[hunyadi]

We are abandoning this pull request in favor of an implementation where DEFAULT constraints in Microsoft SQL Server have a suffix appended with randomly generated characters. This avoids inconsistency when a table is moved or renamed. We have extended database object discovery to read this globally unique ID when constructing the internal model. The constraint name for DEFAULT doesn't participate in equality comparisons: two models are considered equivalent if there is a mismatch only in the name of the DEFAULT constraint.