AIP-83 Restore uniqueness constraint on logical date and make logical date nullable #46232

vatsrahul1001 · 2025-01-29T09:15:58Z

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

…stronomer/airflow into restore-unique-constraint-logical-date

vatsrahul1001 · 2025-01-29T10:45:46Z

Few tests are failing looking into it

…que-constraint-logical-date

…stronomer/airflow into restore-unique-constraint-logical-date

vatsrahul1001 · 2025-01-30T17:56:30Z

Failing test cases will require change in backfill reprocess behaviour, I have create PR which will fix these failing tests

uranusjr · 2025-02-03T06:02:55Z

What’s the order we should merge the two PRs? Tests are not passing on this one, but the other PR needs this to be merged first.

--

Edit: Talked to Rahul, this should be made greeen and go in first.

…que-constraint-logical-date

Lee-W · 2025-02-03T11:00:36Z

airflow/models/dagrun.py

+                or (
+                    task.start_date is None
+                    or (self.logical_date is not None and task.start_date <= self.logical_date)
+                )
+                and (
+                    task.end_date is None
+                    or (self.logical_date is not None and self.logical_date <= task.end_date)
+                )


Suggested change

or (

task.start_date is None

or (self.logical_date is not None and task.start_date <= self.logical_date)

)

and (

task.end_date is None

or (self.logical_date is not None and self.logical_date <= task.end_date)

)

or (

task.start_date is None

or self.logical_date is None

or task.start_date <= self.logical_date

)

and (task.end_date is None or self.logical_date is None or self.logical_date <= task.end_date)

I suspect this "might" be the correct condition.

Yes this one is correct. Thanks @Lee-W

vatsrahul1001 · 2025-02-03T12:24:17Z

What’s the order we should merge the two PRs? Tests are not passing on this one, but the other PR needs this to be merged first.

--

Edit: Talked to Rahul, this should be made greeen and go in first.

@uranusjr Fixed failing tests now only failing tests are now related to backfill test_reprocess_behavior which are fixed in #46295 .

…stronomer/airflow into restore-unique-constraint-logical-date

uranusjr · 2025-02-04T03:17:12Z

airflow/migrations/versions/0032_3_0_0_rename_execution_date_to_logical_date_and_nullable.py

@@ -49,10 +49,15 @@ def upgrade():
            "execution_date",
            new_column_name="logical_date",
            existing_type=TIMESTAMP(timezone=True),
-            existing_nullable=False,
+            existing_nullable=True,


Suggested change

existing_nullable=True,

nullable=True,

existing_nullable is for specifying the nullable property we don’t want to change. Here we want to change nullability.

Thanks, I Implemented this change.

uranusjr · 2025-02-04T03:18:52Z

airflow/migrations/versions/0032_3_0_0_rename_execution_date_to_logical_date_and_nullable.py

@@ -63,7 +68,9 @@ def downgrade():
            existing_type=TIMESTAMP(timezone=True),
            existing_nullable=False,


Suggested change

existing_nullable=False,

nullable=False,

Thanks, I Implemented this change.

uranusjr · 2025-02-04T03:19:37Z

airflow/models/dag.py

+        if logical_date is not None:
+            logical_date = timezone.coerce_datetime(logical_date)


Suggested change

if logical_date is not None:

logical_date = timezone.coerce_datetime(logical_date)

logical_date = timezone.coerce_datetime(logical_date)

This function handles None on its own

Thanks, I Implemented this change.

uranusjr · 2025-02-04T03:41:42Z

tests/models/test_dagrun.py

-        dr2 = dag_maker.create_dagrun_after(
-            dr,
+        next_date = DEFAULT_DATE + datetime.timedelta(days=1)
+        dr2 = dag_maker.create_dagrun(
            run_id="test_dagrun_no_deadlock_2",
            start_date=DEFAULT_DATE + datetime.timedelta(days=1),
+            logical_date=next_date,
        )


Hmm, this shouldn’t need to be changed. How does this fail originally?

@uranusjr When we pass the dr object to create_dagrun_after, it fails with a unique constraint error on the dag_run table. In dag_maker, we set logical_date to start_date if it's None here, and we are passing the same logical_date in create_dagrun_after due to which it fails.

Lee-W

test is still failing 🤔 Please re-request for review once fixed. Thanks!

vatsrahul1001 · 2025-02-04T06:28:09Z

@uranusjr Fixed failing tests now only failing tests are now related to backfill test_reprocess_behavior which are fixed in #46295 .

test is still failing 🤔 Please re-request for review once fixed. Thanks!

@Lee-W as mentioned here, failing tests are now related to backfill test_reprocess_behavior which are fixed in #46295 .

vatsrahul1001 · 2025-02-04T06:51:53Z

Discussed with @Lee-W as #46295 is based on this PR and have changes with CI green. Its better to close this PR in favour of #46295

restore unique constraint on logical date and make it nullable

d0e22ad

vatsrahul1001 requested review from ephraimbuddy, potiuk, XD-DENG and ashb as code owners January 29, 2025 09:15

boring-cyborg bot added area:db-migrations PRs with DB migration kind:documentation labels Jan 29, 2025

vatsrahul1001 marked this pull request as draft January 29, 2025 09:29

vatsrahul1001 and others added 6 commits January 29, 2025 15:06

restore unique constraint on logical date and make it nullable

2b11aed

fix migration file

237360d

Merge branch 'main' into restore-unique-constraint-logical-date

6f423c1

fix migration file

18efafb

Merge branch 'restore-unique-constraint-logical-date' of github.com:a…

8f9e076

…stronomer/airflow into restore-unique-constraint-logical-date

Merge branch 'main' into restore-unique-constraint-logical-date

1c47c75

vatsrahul1001 marked this pull request as ready for review January 29, 2025 10:16

vatsrahul1001 marked this pull request as draft January 29, 2025 10:42

vatsrahul1001 added 3 commits January 30, 2025 08:38

Merge branch 'main' of github.com:astronomer/airflow into restore-uni…

d3bfe20

…que-constraint-logical-date

Merge branch 'restore-unique-constraint-logical-date' of github.com:a…

0fa2477

…stronomer/airflow into restore-unique-constraint-logical-date

fixing tests

75a684a

vatsrahul1001 marked this pull request as ready for review January 30, 2025 15:57

vatsrahul1001 mentioned this pull request Jan 30, 2025

AIP-83: Restore Uniqueness Constraint on Logical Date, Make It Nullable #46295

Merged

vatsrahul1001 requested a review from dstandish January 30, 2025 16:20

sunank200 mentioned this pull request Jan 31, 2025

Make logical date nullable and restore the uniqueness constraint on logical date #45956

Closed

Lee-W self-requested a review February 3, 2025 00:48

vatsrahul1001 added 2 commits February 3, 2025 10:10

resolve conflict

53d329b

remove default date from logical date in dag run model

c3e29c7

Merge branch 'main' of github.com:astronomer/airflow into restore-uni…

0830d58

…que-constraint-logical-date

Lee-W reviewed Feb 3, 2025

View reviewed changes

vatsrahul1001 and others added 3 commits February 3, 2025 16:34

fix task_filter

2b4251d

merge main

3ec1ecd

Merge branch 'main' into restore-unique-constraint-logical-date

5680a76

vatsrahul1001 requested a review from Lee-W February 3, 2025 12:45

vatsrahul1001 and others added 3 commits February 3, 2025 18:29

Merge branch 'main' into restore-unique-constraint-logical-date

16cd717

fix tests

7d6c9f4

Merge branch 'restore-unique-constraint-logical-date' of github.com:a…

02a82a0

…stronomer/airflow into restore-unique-constraint-logical-date

vatsrahul1001 mentioned this pull request Feb 3, 2025

AIP-83 Logical date should be required field when triggering run via API #46390

Open

uranusjr reviewed Feb 4, 2025

View reviewed changes

implement review comments

83b62bb

vatsrahul1001 requested a review from uranusjr February 4, 2025 04:47

Lee-W reviewed Feb 4, 2025

View reviewed changes

vatsrahul1001 requested a review from Lee-W February 4, 2025 06:34

vatsrahul1001 closed this Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AIP-83 Restore uniqueness constraint on logical date and make logical date nullable #46232

AIP-83 Restore uniqueness constraint on logical date and make logical date nullable #46232

vatsrahul1001 commented Jan 29, 2025 •

edited

Loading

vatsrahul1001 commented Jan 29, 2025

vatsrahul1001 commented Jan 30, 2025

uranusjr commented Feb 3, 2025 •

edited

Loading

Lee-W Feb 3, 2025

Lee-W Feb 3, 2025

vatsrahul1001 Feb 3, 2025

vatsrahul1001 commented Feb 3, 2025 •

edited

Loading

uranusjr Feb 4, 2025

vatsrahul1001 Feb 4, 2025

uranusjr Feb 4, 2025

vatsrahul1001 Feb 4, 2025

uranusjr Feb 4, 2025

vatsrahul1001 Feb 4, 2025

uranusjr Feb 4, 2025

vatsrahul1001 Feb 4, 2025

Lee-W left a comment

vatsrahul1001 commented Feb 4, 2025

vatsrahul1001 commented Feb 4, 2025

		@@ -63,7 +68,9 @@ def downgrade():
		existing_type=TIMESTAMP(timezone=True),
		existing_nullable=False,

		if logical_date is not None:
		logical_date = timezone.coerce_datetime(logical_date)

AIP-83 Restore uniqueness constraint on logical date and make logical date nullable #46232

AIP-83 Restore uniqueness constraint on logical date and make logical date nullable #46232

Conversation

vatsrahul1001 commented Jan 29, 2025 • edited Loading

vatsrahul1001 commented Jan 29, 2025

vatsrahul1001 commented Jan 30, 2025

uranusjr commented Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vatsrahul1001 commented Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lee-W left a comment

Choose a reason for hiding this comment

vatsrahul1001 commented Feb 4, 2025

vatsrahul1001 commented Feb 4, 2025

vatsrahul1001 commented Jan 29, 2025 •

edited

Loading

uranusjr commented Feb 3, 2025 •

edited

Loading

vatsrahul1001 commented Feb 3, 2025 •

edited

Loading