Add server default for map_index in Log table

[dstandish]

When logging CLI actions we insert a record into the Log table. But for 2.3 we add column map_index to Log, and if the Log model expects map_index to be there the insert will fail and a warning will be emitted.

We can avoid the error and warning by adding a server_default of NULL on map_index in Log. I choose NULL instead of -1 because generally speaking map_index doesn't make sense for Log tables.

[github-actions]

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

[uranusjr]

I think this needs a change in the migration file as well?

[dstandish]

I think this needs a change in the migration file as well?

@uranusjr in the migration file it's nullable which means essentially that the server default is NULL already. So I don't think we need to change anything more but let me know if you think there's something we need to do.

[uranusjr]

in the migration file it's nullable which means essentially that the server default is NULL already.

But the definition in the class also declares nullable (in the sense that nullable is not False and True is the default). From what I understand, server_default is injected into ALTER TABLE when the table is being modified (or CREATE TABLE), and without adding this to a migration this would do nothing.

Instead of server_default perhaps we need to add self.map_index = None in __init__ instead.

[dstandish]

From what I understand, server_default is injected into ALTER TABLE when the table is being modified (or CREATE TABLE), and without adding this to a migration this would do nothing.

So it does something -- namely, it seems to make it so that when it generates an insert, it doesn't include map_index as a column when it's not set. I learned of the behavior here. And I did test it locally. I don't think adding to migration would change the behavior (since NULL is already the server default, right?) but it's easy enough to do so I'll add that in a sec.

Instead of server_default perhaps we need to add self.map_index = None in __init__ instead.

I just tried this and it doesn't work. I suspect it already gets a None value since it doesn't get set in init.

You can test locally on main by taking a fresh db, dropping map_index from log table, and running db upgrade.