mattiabasone commented on code in PR #3528:
URL: https://github.com/apache/avro/pull/3528#discussion_r2494970642


##########
lang/php/lib/Datum/AvroIOBinaryEncoder.php:
##########
@@ -172,30 +155,26 @@ public static function doubleToLongBits($double): string
     }
 
     /**
-     * @param string $str
      * @uses self::writeBytes()
      */
-    public function writeString($str): void
+    public function writeString(string $str): void
     {
         $this->writeBytes($str);
     }
 
-    /**
-     * @param string $bytes
-     */
-    public function writeBytes($bytes): void
+    public function writeBytes(string $bytes): void
     {
         $this->writeLong(strlen($bytes));
         $this->write($bytes);
     }
 
-    public function writeDecimal($decimal, int $scale, int $precision): void
+    public function writeDecimal(string $decimal, int $scale, int $precision): 
void
     {
         if (!is_numeric($decimal)) {
             throw new AvroException("Decimal value '{$decimal}' must be 
numeric");
         }
 
-        $value = $decimal * (10 ** $scale);
+        $value = ((float) $decimal) * (10 ** $scale);

Review Comment:
   The internal type for Float an Double is the same, for the Decimal we are 
using strings and then apply the logic in the encoder above.
   
   I agree that we should use `brick/math` library, maybe it's something that 
can be addressed in another PR, there are already a lot of changes here :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to