Improve performance of comparison transformations (#1739)

norberttech · web-flow · commit 1f550f0293f8 · 2025-06-26T13:37:37.000-05:00
diff --git a/shell.nix b/shell.nix
@@ -1,6 +1,7 @@
 let
     nixpkgs = fetchTarball {
-        url = "https://github.com/NixOS/nixpkgs/archive/d2e52032da935f4972579f132250f3c3f47136d1.tar.gz";
+        url = "https://github.com/NixOS/nixpkgs/archive/88a5945d774ff733c2ecd65247aba714a1731fe7.tar.gz";
+
     };
 
     lockedPkgs = import nixpkgs {
diff --git a/src/core/etl/src/Flow/ETL/Function/Equals.php b/src/core/etl/src/Flow/ETL/Function/Equals.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '==');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '==');
 
         return $left == $right;
     }
diff --git a/src/core/etl/src/Flow/ETL/Function/GreaterThan.php b/src/core/etl/src/Flow/ETL/Function/GreaterThan.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '>');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '>');
 
         if ($left === null || $right === null) {
             return false;
diff --git a/src/core/etl/src/Flow/ETL/Function/GreaterThanEqual.php b/src/core/etl/src/Flow/ETL/Function/GreaterThanEqual.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '>=');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '>=');
 
         if ($left === null || $right === null) {
             return false;
diff --git a/src/core/etl/src/Flow/ETL/Function/Greatest.php b/src/core/etl/src/Flow/ETL/Function/Greatest.php
@@ -20,16 +20,18 @@ public function __construct(
     public function eval(Row $row) : mixed
     {
         $extractedValues = [];
+        $extractedTypes = [];
 
         foreach ($this->values as $value) {
             $extractedValues[] = (new Parameter($value))->eval($row);
+            $extractedTypes[] = (new Parameter($value))->asType($row);
         }
 
         if (!\count($extractedValues)) {
             return null;
         }
 
-        (new ValueComparator())->assertAllComparable($extractedValues, '>');
+        (new ValueComparator())->assertAllTypesComparable($extractedTypes, '>');
 
         return max($extractedValues);
     }
diff --git a/src/core/etl/src/Flow/ETL/Function/Least.php b/src/core/etl/src/Flow/ETL/Function/Least.php
@@ -20,16 +20,18 @@ public function __construct(
     public function eval(Row $row) : mixed
     {
         $extractedValues = [];
+        $extractedTypes = [];
 
         foreach ($this->values as $value) {
             $extractedValues[] = (new Parameter($value))->eval($row);
+            $extractedTypes[] = (new Parameter($value))->asType($row);
         }
 
         if (!\count($extractedValues)) {
             return null;
         }
 
-        (new ValueComparator())->assertAllComparable($extractedValues, '<');
+        (new ValueComparator())->assertAllTypesComparable($extractedTypes, '<');
 
         return \min($extractedValues);
     }
diff --git a/src/core/etl/src/Flow/ETL/Function/LessThan.php b/src/core/etl/src/Flow/ETL/Function/LessThan.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '<');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '<');
 
         if ($left === null || $right === null) {
             return false;
diff --git a/src/core/etl/src/Flow/ETL/Function/LessThanEqual.php b/src/core/etl/src/Flow/ETL/Function/LessThanEqual.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '<=');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '<=');
 
         if ($left === null || $right === null) {
             return false;
diff --git a/src/core/etl/src/Flow/ETL/Function/NotEquals.php b/src/core/etl/src/Flow/ETL/Function/NotEquals.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '!=');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '!=');
 
         return $left != $right;
     }
diff --git a/src/core/etl/src/Flow/ETL/Function/NotSame.php b/src/core/etl/src/Flow/ETL/Function/NotSame.php
@@ -18,9 +18,11 @@ public function __construct(
     public function eval(Row $row) : bool
     {
         $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
         $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($left, $right, '!==');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '!==');
 
         return $left !== $right;
     }
diff --git a/src/core/etl/src/Flow/ETL/Function/Parameter.php b/src/core/etl/src/Flow/ETL/Function/Parameter.php
@@ -5,6 +5,7 @@
 namespace Flow\ETL\Function;
 
 use function Flow\ETL\DSL\lit;
+use function Flow\Types\DSL\get_type;
 use Flow\ETL\Function\ScalarFunction\ScalarResult;
 use Flow\ETL\Row;
 use Flow\ETL\Row\{Entry, Reference};
@@ -171,6 +172,20 @@ public function asString(Row $row, ?string $default = null) : ?string
         return \is_string($result) ? $result : $default;
     }
 
+    /**
+     * @return Type<mixed>
+     */
+    public function asType(Row $row) : Type
+    {
+        if ($this->function instanceof Reference) {
+            return $row->get($this->function)->type();
+        }
+
+        $result = $this->eval($row);
+
+        return get_type($result);
+    }
+
     public function eval(Row $row) : mixed
     {
         $result = $this->function->eval($row);
diff --git a/src/core/etl/src/Flow/ETL/Function/Same.php b/src/core/etl/src/Flow/ETL/Function/Same.php
@@ -17,11 +17,13 @@ public function __construct(
 
     public function eval(Row $row) : bool
     {
-        $base = (new Parameter($this->left))->eval($row);
-        $next = (new Parameter($this->right))->eval($row);
+        $left = (new Parameter($this->left))->eval($row);
+        $leftType = (new Parameter($this->left))->asType($row);
+        $right = (new Parameter($this->right))->eval($row);
+        $rightType = (new Parameter($this->right))->asType($row);
 
-        (new ValueComparator())->assertComparable($base, $next, '===');
+        (new ValueComparator())->assertComparableTypes($leftType, $rightType, '===');
 
-        return $base === $next;
+        return $left === $right;
     }
 }
diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/ParameterTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/Function/ParameterTest.php
@@ -4,7 +4,7 @@
 
 namespace Flow\ETL\Tests\Unit\Function;
 
-use function Flow\ETL\DSL\{lit, ref, row, str_entry};
+use function Flow\ETL\DSL\{int_entry, lit, ref, row, str_entry};
 use function Flow\Types\DSL\{type_boolean, type_integer, type_string};
 use Flow\ETL\Function\Parameter;
 use Flow\ETL\Function\ScalarFunction\ScalarResult;
@@ -43,4 +43,48 @@ public function test_as_scalar_on_scalar_result() : void
         self::assertNull($parameter->as(row(), type_integer()));
         self::assertSame('test', $parameter->as(row(), type_string()));
     }
+
+    public function test_as_type_with_literal_value() : void
+    {
+        $parameter = new Parameter(lit('string_value'));
+        $type = $parameter->asType(row());
+        self::assertTrue($type->isValid('string_value'));
+        self::assertFalse($type->isValid(42));
+
+        $parameter = new Parameter(lit(42));
+        $type = $parameter->asType(row());
+        self::assertTrue($type->isValid(42));
+        self::assertFalse($type->isValid('string'));
+
+        $parameter = new Parameter(lit(true));
+        $type = $parameter->asType(row());
+        self::assertTrue($type->isValid(true));
+        self::assertFalse($type->isValid(42));
+    }
+
+    public function test_as_type_with_reference() : void
+    {
+        $parameter = new Parameter(ref('value'));
+
+        $type = $parameter->asType(row(str_entry('value', 'test')));
+        self::assertTrue($type->isValid('test'));
+        self::assertFalse($type->isValid(42));
+
+        $type = $parameter->asType(row(int_entry('value', 42)));
+        self::assertTrue($type->isValid(42));
+        self::assertFalse($type->isValid('test'));
+    }
+
+    public function test_as_type_with_scalar_result() : void
+    {
+        $parameter = new Parameter(lit(ScalarResult::from('test')));
+        $type = $parameter->asType(row());
+        self::assertTrue($type->isValid('test'));
+        self::assertFalse($type->isValid(42));
+
+        $parameter = new Parameter(lit(ScalarResult::from(123)));
+        $type = $parameter->asType(row());
+        self::assertTrue($type->isValid(123));
+        self::assertFalse($type->isValid('string'));
+    }
 }
diff --git a/src/lib/parquet/src/Flow/Parquet/ParquetFile/RowGroupBuilder/ColumnData/FlatColumnValues.php b/src/lib/parquet/src/Flow/Parquet/ParquetFile/RowGroupBuilder/ColumnData/FlatColumnValues.php
@@ -103,7 +103,15 @@ public function nullCount() : int
     {
         $maxDefinitionLevel = $this->column->repetitions()->maxDefinitionLevel();
 
-        return \count(\array_filter($this->definitionLevels, fn (int $d) => $d !== $maxDefinitionLevel));
+        $nullCount = 0;
+
+        foreach ($this->definitionLevels as $definitionLevel) {
+            if ($definitionLevel !== $maxDefinitionLevel) {
+                $nullCount++;
+            }
+        }
+
+        return $nullCount;
     }
 
     /**
@@ -116,8 +124,15 @@ public function repetitionLevels() : array
 
     public function rowsCount() : int
     {
-        // rows count is count of repetitions equal to 0
-        return \count(\array_filter($this->repetitionLevels, static fn (int $r) => $r === 0));
+        $rowsCount = 0;
+
+        foreach ($this->repetitionLevels as $repetitionLevel) {
+            if ($repetitionLevel === 0) {
+                $rowsCount++;
+            }
+        }
+
+        return $rowsCount;
     }
 
     public function skipRows(?int $skipRows) : self
diff --git a/src/lib/types/src/Flow/Types/Type/Comparator.php b/src/lib/types/src/Flow/Types/Type/Comparator.php
@@ -6,7 +6,14 @@
 
 use function Flow\Types\DSL\{type_equals, type_instance_of};
 use Flow\Types\Type;
-use Flow\Types\Type\Logical\{DateTimeType, DateType, ListType, MapType, OptionalType, StructureType, TimeType};
+use Flow\Types\Type\Logical\{DateTimeType,
+    DateType,
+    JsonType,
+    ListType,
+    MapType,
+    OptionalType,
+    StructureType,
+    TimeType};
 use Flow\Types\Type\Native\{FloatType, IntegerType, NullType, StringType, UnionType};
 
 final class Comparator
@@ -57,6 +64,10 @@ public function comparable(Type $left, Type $right) : bool
             return true;
         }
 
+        if (\in_array($left::class, [StringType::class, JsonType::class], true) && \in_array($right::class, [StringType::class, JsonType::class], true)) {
+            return true;
+        }
+
         return type_equals($left, $right);
     }
 
diff --git a/src/lib/types/src/Flow/Types/Type/ValueComparator.php b/src/lib/types/src/Flow/Types/Type/ValueComparator.php
@@ -5,31 +5,22 @@
 namespace Flow\Types\Type;
 
 use Flow\Types\Exception\InvalidArgumentException;
+use Flow\Types\Type;
 use Flow\Types\Type\Comparison\Operator;
 
 final readonly class ValueComparator
 {
-    public function __construct(private Comparator $comparator = new Comparator(), private TypeDetector $detector = new TypeDetector())
+    public function __construct(private Comparator $comparator = new Comparator())
     {
     }
 
     /**
-     * @param array<array-key, mixed> $values
+     * @param array<array-key, Type<mixed>> $types
      */
-    public function assertAllComparable(array $values, Operator|string $operator) : void
+    public function assertAllTypesComparable(array $types, Operator|string $operator) : void
     {
         $operator = \is_string($operator) ? Operator::from($operator) : $operator;
 
-        $types = [];
-
-        foreach ($values as $value) {
-            $type = $this->detector->detectType($value);
-
-            if (!in_array($type, $types, true)) {
-                $types[] = $type;
-            }
-        }
-
         if (count($types) > 1) {
             foreach ($types as $nextType) {
                 foreach ($types as $baseType) {
@@ -41,15 +32,16 @@ public function assertAllComparable(array $values, Operator|string $operator) :
         }
     }
 
-    public function assertComparable(mixed $left, mixed $right, Operator|string $operator) : void
+    /**
+     * @param Type<mixed> $left
+     * @param Type<mixed> $right
+     */
+    public function assertComparableTypes(Type $left, Type $right, Operator|string $operator) : void
     {
         $operator = \is_string($operator) ? Operator::from($operator) : $operator;
 
-        $baseType = $this->detector->detectType($left);
-        $nextType = $this->detector->detectType($right);
-
-        if (!$this->comparator->comparable($baseType, $nextType)) {
-            throw new InvalidArgumentException(\sprintf("Can't compare '(%s %s %s)' due to data type mismatch.", $baseType->toString(), $operator->value, $nextType->toString()));
+        if (!$this->comparator->comparable($left, $right)) {
+            throw new InvalidArgumentException(\sprintf("Can't compare '(%s %s %s)' due to data type mismatch.", $left->toString(), $operator->value, $right->toString()));
         }
     }
 }
diff --git a/src/lib/types/tests/Flow/Types/Tests/Unit/Type/ComparatorTest.php b/src/lib/types/tests/Flow/Types/Tests/Unit/Type/ComparatorTest.php
@@ -10,6 +10,7 @@
     type_integer,
     type_is,
     type_is_any,
+    type_json,
     type_list,
     type_map,
     type_null,
@@ -29,6 +30,7 @@ final class ComparatorTest extends TestCase
     public static function type_comparable_data_provider() : \Generator
     {
         yield [type_integer(), type_integer()];
+        yield [type_json(), type_string()];
         yield [type_integer(), type_float()];
         yield [type_float(), type_integer()];
         yield [type_float(), type_float()];
diff --git a/src/lib/types/tests/Flow/Types/Tests/Unit/Type/ValueComparatorTest.php b/src/lib/types/tests/Flow/Types/Tests/Unit/Type/ValueComparatorTest.php

Original file line number	Diff line number	Diff line change
`@@ -18,9 +18,11 @@ public function __construct(`
`18`	`18`	`public function eval(Row $row) : bool`
`19`	`19`	`{`
`20`	`20`	`$left = (new Parameter($this->left))->eval($row);`
	`21`	`+ $leftType = (new Parameter($this->left))->asType($row);`
`21`	`22`	`$right = (new Parameter($this->right))->eval($row);`
	`23`	`+ $rightType = (new Parameter($this->right))->asType($row);`
`22`	`24`
`23`		`- (new ValueComparator())->assertComparable($left, $right, '==');`
	`25`	`+ (new ValueComparator())->assertComparableTypes($leftType, $rightType, '==');`
`24`	`26`
`25`	`27`	`return $left == $right;`
`26`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,16 +20,18 @@ public function __construct(`
`20`	`20`	`public function eval(Row $row) : mixed`
`21`	`21`	`{`
`22`	`22`	`$extractedValues = [];`
	`23`	`+ $extractedTypes = [];`
`23`	`24`
`24`	`25`	`foreach ($this->values as $value) {`
`25`	`26`	`$extractedValues[] = (new Parameter($value))->eval($row);`
	`27`	`+ $extractedTypes[] = (new Parameter($value))->asType($row);`
`26`	`28`	`}`
`27`	`29`
`28`	`30`	`if (!\count($extractedValues)) {`
`29`	`31`	`return null;`
`30`	`32`	`}`
`31`	`33`
`32`		`- (new ValueComparator())->assertAllComparable($extractedValues, '>');`
	`34`	`+ (new ValueComparator())->assertAllTypesComparable($extractedTypes, '>');`
`33`	`35`
`34`	`36`	`return max($extractedValues);`
`35`	`37`	`}`