Reduce db functions to one SQLAlchemy operation per function

[silentninja]

Problem

We have decided to use attnums when passing a column reference as arguments but most of the SQLAlchemy operations use column name, so we need to fetch the column name before an SQLAlchemy operation, this can lead to referencing a wrong column in case there are multiple SQLAlchemny operations within a single function and one of those happens to change a column name.

Proposed solution

Each function should use only one SQLAlchemy operation so a column name altering operation won't have any effect on other operations.

[vrutik2809]

@silentninja I would like to work on this issue. Can you please elaborate on this issue and how to get started?

[pavish]

@vrutik2809 I've assigned this issue to you. @silentninja will get back to you when he's back online. In the meanwhile, please feel free to ask your questions directly on our matrix channels.

[vrutik2809]

Thank you

[silentninja]

@vrutik2809 I think this issue needs some in-depth knowledge about the codebase, I was wrong about tagging this as a good first issue. I have unassigned you, Please feel free to work on some other issue. Really sorry for the inconvenience

[vrutik2809]

No problem. I will look for another issue

Thanks

[Anish9901]

@silentninja In the function alter_column inside db/columns/operations/alter.py each method are given column_attnum and those methods get the name of the column using attnum how can this lead to referencing a wrong column?

[silentninja]

Quoted from the proposed solution

Each function should use only one SQLAlchemy operation

@Anish9901 Function alter_column does not call any SQLALchemy operations, it composes of multiple functions which take attnum as parameters and does a single SQL operation, essentially acting as a attnum wrapper for a SQL operation, such functions don't have to be changed.

This issue is about splitting the function that uses multiple SQL operations(using SQL alchemy). For example, db.columns.operations.alter._batch_alter_table_columns calls multiple SQLAlchemy operation within it, https://github.com/centerofci/mathesar/blob/e2ab941c4ade4737f4e14ed72b8cb3ee67f341ac/db/columns/operations/alter.py#L213 https://github.com/centerofci/mathesar/blob/e2ab941c4ade4737f4e14ed72b8cb3ee67f341ac/db/columns/operations/alter.py#L218

[Anish9901]

@silentninja I have understood the issue, I would like to work on fixing it, can you please assign me?

[seancolsen]

Thanks @Anish9901 I've assigned this to you.

[Anish9901]

@silentninja I have made a PR #1266 regarding this issue please take a look.

[kgodey]

@Anish9901 are you still working on this?

[Anish9901]

Yes, @kgodey the only thing left to add are test cases, as @mathemancer said it could be fairly technical to write them, I'll work on it ASAP.

[mathemancer]

Given that we're planning to go a different direction which will change how we use SQLAlchemy, I think we can consider this issue resolved by @Anish9901 's PR #1266 for now.