fix(taxonomy): don't convert numeric strings to numbers (#19774)

PostHog · Jan 22, 2024 · 4f50326 · 4f50326
1 parent 5ea8a3e
commit 4f50326
Show file tree

Hide file tree

Showing 5 changed files with 307 additions and 322 deletions.
diff --git a/plugin-server/functional_tests/definitions.test.ts b/plugin-server/functional_tests/definitions.test.ts
@@ -57,8 +57,8 @@ test.concurrent.each([[2], [2.1234], ['2'], ['2.1234']])(
             expect(propertyDefinitions).toContainEqual(
                 expect.objectContaining({
                     name: 'property',
-                    is_numerical: true,
-                    property_type: 'Numeric',
+                    is_numerical: typeof numberValue === 'number',
+                    property_type: typeof numberValue === 'number' ? 'Numeric' : 'String',
                 })
             )
         })

diff --git a/plugin-server/src/worker/ingestion/property-definitions-auto-discovery.ts b/plugin-server/src/worker/ingestion/property-definitions-auto-discovery.ts
@@ -3,10 +3,6 @@ import { DateTimePropertyTypeFormat, PropertyType, UnixTimestampPropertyTypeForm
 // magic copied from https://stackoverflow.com/a/54930905
 // allows candidate to be typed as any
 
-export const isNumericString = (candidate: any): boolean => {
-    return !(candidate instanceof Array) && candidate - parseFloat(candidate) + 1 >= 0
-}
-
 export const unixTimestampPropertyTypeFormatPatterns: Record<keyof typeof UnixTimestampPropertyTypeFormat, RegExp> = {
     UNIX_TIMESTAMP: /^\d{10}(\.\d*)?$/,
     UNIX_TIMESTAMP_MILLISECONDS: /^\d{13}$/,
@@ -106,18 +102,12 @@ export const detectPropertyDefinitionTypes = (value: unknown, key: string): Prop
     if (typeof value === 'string') {
         propertyType = PropertyType.String
 
-        if (isNumericString(value)) {
-            propertyType = PropertyType.Numeric
-        }
-
         Object.values(dateTimePropertyTypeFormatPatterns).find((pattern) => {
             if (value.match(pattern)) {
                 propertyType = PropertyType.DateTime
                 return true
             }
         })
-
-        detectUnixTimestamps()
     }
 
     if (