Postgres function should_update_person_props

latest_migrations.manifest

@@ -3,7 +3,7 @@ auth: 0012_alter_user_first_name_max_length
 axes: 0006_remove_accesslog_trusted
 contenttypes: 0002_remove_content_type_name
 ee: 0005_project_based_permissioning
-posthog: 0172_person_properties_last_operation
+posthog: 0173_should_update_person_props_function
 rest_hooks: 0002_swappable_hook_model
 sessions: 0001_initial
-social_django: 0010_uid_db_index
+social_django: 0010_uid_db_index

posthog/management/commands/test_migrations_are_null.py

@@ -15,7 +15,7 @@ def run_and_check_migration(variable):
             try:
                 results = re.findall(r"([a-z]+)\/migrations\/([a-zA-Z_0-9]+)\.py", variable)[0]
                 sql = call_command("sqlmigrate", results[0], results[1])
-                if "NOT NULL" in sql and "Create model" not in sql:
+                if "NOT NULL" in sql and "Create model" not in sql and "-- not-null-ignore" not in sql:
                     print(
                         f"\n\n\033[91mFound a non-null field added to an existing model. This will lock up the table while migrating. Please add 'null=True, blank=True' to the field",
                         "red",

posthog/migrations/0173_should_update_person_props_function.py

@@ -0,0 +1,39 @@
+# Generated by Django 3.1.12 on 2021-10-05 13:06
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("posthog", "0172_person_properties_last_operation"),
+    ]
+
+    operations = [
+        migrations.RunSQL(
+            """
+        -- not-null-ignore
+        CREATE OR REPLACE FUNCTION should_update_person_prop(
+            in person_id int, 
+            in prop_key text, 
+            in _timestamp text, 
+            in operation text, 
+            out should_update bool
+        )
+        AS $$ 
+            SELECT NOT property_exists OR
+            (operation='set' AND _timestamp > stored_timestamp) OR
+            (last_operation='set_once' AND _timestamp < stored_timestamp)
+            FROM (
+                SELECT 
+                    properties->prop_key IS NOT NULL as property_exists, 
+                    COALESCE(properties_last_updated_at ->> prop_key, '0') as stored_timestamp,
+                    COALESCE(properties_last_operation ->> prop_key, 'set') as last_operation 
+                    FROM posthog_person
+                    WHERE id=person_id
+            ) as person_props
+        $$
+        LANGUAGE SQL;
+        """
+        )
+    ]

posthog/test/test_should_update_person_prop.py

@@ -0,0 +1,239 @@
+from datetime import datetime
+
+from django.db import connection
+
+from posthog.models import Person
+from posthog.test.base import BaseTest
+
+# How we expect this function to behave:
+#   | call     | value exists  | call TS is ___ existing TS | previous fn | write/override
+#  1| set      | no            | N/A                        | N/A         | yes
+#  2| set_once | no            | N/A                        | N/A         | yes
+#  3| set      | yes           | before                     | set         | no
+#  4| set      | yes           | before                     | set_once    | yes
+#  5| set      | yes           | after                      | set         | yes
+#  6| set      | yes           | after                      | set_once    | yes
+#  7| set_once | yes           | before                     | set         | no
+#  8| set_once | yes           | before                     | set_once    | yes
+#  9| set_once | yes           | after                      | set         | no
+# 10| set_once | yes           | after                      | set_once    | no
+# 11| set      | yes           | equal                      | set         | no
+# 12| set_once | yes           | equal                      | set         | no
+# 13| set      | yes           | equal                      | set_once    | no
+# 14| set_once | yes           | equal                      | set_once    | no
+
+# Refers to migration 0173_should_update_person_props_function
+# This is a Postgres function we use in the plugin server
+class TestShouldUpdatePersonProp(BaseTest):
+    def test_update_without_properties_last_updated_at(self):
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={},
+            properties_last_operation={"a": "set", "b": "set_once"},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'non-existent prop', now()::text, 'set'),
+                    should_update_person_prop({person.id}, 'non-existent prop', now()::text, 'set_once')
+            """
+            )
+
+            result = cursor.fetchall()
+            set_op_result = result[0][0]
+            set_once_op_result = result[0][1]
+
+            self.assertEqual(set_op_result, True)
+            self.assertEqual(set_once_op_result, True)
+
+    def test_update_without_properties_last_operation(self):
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={
+                "a": datetime(2050, 1, 1, 1, 1, 1).isoformat(),
+                "b": datetime(2050, 1, 1, 1, 1, 1).isoformat(),
+            },
+            properties_last_operation={},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'non-existent prop', now()::text, 'set'),
+                    should_update_person_prop({person.id}, 'non-existent prop', now()::text, 'set_once')
+            """
+            )
+
+            result = cursor.fetchall()
+            set_op_result = result[0][0]
+            set_once_op_result = result[0][1]
+
+            self.assertEqual(set_op_result, True)
+            self.assertEqual(set_once_op_result, True)
+
+    # tests cases 1 and 2 from the table
+    def test_update_non_existent_prop(self):
+        person = Person.objects.create(
+            team=self.team, properties={}, properties_last_updated_at={}, properties_last_operation={}
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'non-existent prop', now()::text, 'set'),
+                    should_update_person_prop({person.id}, 'non-existent prop', now()::text, 'set_once')
+            """
+            )
+
+            result = cursor.fetchall()
+            set_op_result = result[0][0]
+            set_once_op_result = result[0][1]
+
+            self.assertEqual(set_op_result, True)
+            self.assertEqual(set_once_op_result, True)
+
+    # tests cases 3 and 4 from the table
+    def test_set_operation_with_earlier_timestamp(self):
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={
+                "a": datetime(2050, 1, 1, 1, 1, 1).isoformat(),
+                "b": datetime(2050, 1, 1, 1, 1, 1).isoformat(),
+            },
+            properties_last_operation={"a": "set", "b": "set_once"},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'a', now()::text, 'set'),
+                    should_update_person_prop({person.id}, 'b', now()::text, 'set')
+            """
+            )
+
+            result = cursor.fetchall()
+            previous_op_set_result = result[0][0]
+            previous_op_set_once_result = result[0][1]
+
+            self.assertEqual(previous_op_set_result, False)
+            self.assertEqual(previous_op_set_once_result, True)
+
+    # tests cases 5 and 6 from the table
+    def test_set_operation_with_older_timestamp(self):
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={
+                "a": datetime(2000, 1, 1, 1, 1, 1).isoformat(),
+                "b": datetime(2000, 1, 1, 1, 1, 1).isoformat(),
+            },
+            properties_last_operation={"a": "set", "b": "set_once"},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'a', now()::text, 'set'),
+                    should_update_person_prop({person.id}, 'b', now()::text, 'set')
+            """
+            )
+
+            result = cursor.fetchall()
+            previous_op_set_result = result[0][0]
+            previous_op_set_once_result = result[0][1]
+
+            self.assertEqual(previous_op_set_result, True)
+            self.assertEqual(previous_op_set_once_result, True)
+
+    # tests cases 7 and 8 from the table
+    def test_set_once_operation_with_earlier_timestamp(self):
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={
+                "a": datetime(2050, 1, 1, 1, 1, 1).isoformat(),
+                "b": datetime(2050, 1, 1, 1, 1, 1).isoformat(),
+            },
+            properties_last_operation={"a": "set", "b": "set_once"},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'a', now()::text, 'set_once'),
+                    should_update_person_prop({person.id}, 'b', now()::text, 'set_once')
+            """
+            )
+
+            result = cursor.fetchall()
+            previous_op_set_result = result[0][0]
+            previous_op_set_once_result = result[0][1]
+
+            self.assertEqual(previous_op_set_result, False)
+            self.assertEqual(previous_op_set_once_result, True)
+
+    # tests cases 9 and 10 from the table
+    def test_set_once_operation_with_older_timestamp(self):
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={
+                "a": datetime(2000, 1, 1, 1, 1, 1).isoformat(),
+                "b": datetime(2000, 1, 1, 1, 1, 1).isoformat(),
+            },
+            properties_last_operation={"a": "set", "b": "set_once"},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'a', now()::text, 'set_once'),
+                    should_update_person_prop({person.id}, 'b', now()::text, 'set_once')
+            """
+            )
+
+            result = cursor.fetchall()
+            previous_op_set_result = result[0][0]
+            previous_op_set_once_result = result[0][1]
+
+            self.assertEqual(previous_op_set_result, False)
+            self.assertEqual(previous_op_set_once_result, False)
+
+    # tests cases 11-14 from the table
+    def test_equal_timestamps(self):
+        timestamp = datetime(2000, 1, 1, 1, 1, 1).isoformat()
+        person = Person.objects.create(
+            team=self.team,
+            properties={"a": 0, "b": 0},
+            properties_last_updated_at={"a": timestamp, "b": timestamp,},
+            properties_last_operation={"a": "set", "b": "set_once"},
+        )
+
+        with connection.cursor() as cursor:
+            cursor.execute(
+                f"""
+                SELECT 
+                    should_update_person_prop({person.id}, 'a', '{timestamp}', 'set_once'),
+                    should_update_person_prop({person.id}, 'a', '{timestamp}', 'set'),
+                    should_update_person_prop({person.id}, 'b', '{timestamp}', 'set_once'),
+                    should_update_person_prop({person.id}, 'b', '{timestamp}', 'set')
+            """
+            )
+
+            results = cursor.fetchall()[0]
+
+            self.assertEqual(results[0], False)
+            self.assertEqual(results[1], False)
+            self.assertEqual(results[2], False)
+            self.assertEqual(results[3], False)