LuciferYang opened a new pull request, #9563:
URL: https://github.com/apache/paimon/pull/9563

   ### Purpose
   
   close #9562
   
   `BinaryToDecimalUpdater` allocates a scratch `HeapBytesVector` of capacity 1 
and then indexes it with the target row's offset:
   
   ```java
   this.bytesVector = new HeapBytesVector(1);
   ...
   valuesReader.readBinary(1, bytesVector, offset);
   BigInteger value = new BigInteger(bytesVector.getBytes(offset).getBytes());
   ```
   
   `HeapBytesVector.putByteArray` writes `start[elementNum]`, and `start` has 
one slot, so the second row of any non-dictionary page throws 
`ArrayIndexOutOfBoundsException`. Reading and getting at index 0 is consistent 
with the capacity; the target vector is still written at the real `offset` by 
`putDecimal`. The scratch vector was introduced in #5582 together with the 
32-bit and 64-bit decimal targets; before that the read went into the 
batch-sized target vector, where `offset` was in range, which is where the 
index came from.
   
   The Parquet spec allows DECIMAL on BINARY, but Paimon's own writer emits 
INT32, INT64 or FIXED_LEN_BYTE_ARRAY for decimals, so only externally written 
files reach this updater: parquet-avro maps an Avro `bytes` field carrying a 
decimal logical type onto BINARY, which is what Debezium and Kafka Connect 
produce. Paimon reads such files through a format table, and through 
`migrate_table` or clone, which rename the source files rather than rewriting 
them.
   
   ### Tests
   
   `ParquetTypeWideningTest.testBinaryDecimalReadsEveryRowOfAPage` writes a 
three-row file with `optional binary price (DECIMAL(scale=2))` using 
`ExampleParquetWriter` and reads it back through `ParquetReaderFactory`, for 
precision 5 and 20, which covers two of `putDecimal`'s three output branches, 
the int-backed and the byte-backed one.
   
   Two conditions the test has to get right. Dictionary encoding must be off, 
since a dictionary page is decoded by `decodeSingleDictionaryId` and never 
touches the scratch vector, and with the writer default three small decimals do 
end up in a dictionary page; `writeGroups` therefore takes a flag, with the 
existing two-argument callers left on the previous default. And the read type 
has to name the column `price`, because a name mismatch makes the reader fill 
the column with nulls and the test then fails on fixed code too.
   
   Against the unfixed reader it fails with `ArrayIndexOutOfBoundsException: 1` 
in `VectorizedPlainValuesReader.readBinary`, reached through 
`VectorizedParquetRecordReader.nextBatch`, so the test exercises the real 
reader stack rather than the updater in isolation.
   
   `mvn -pl paimon-format test` on JDK 8: 597 tests, 0 failures. 
`spotless:check` and `checkstyle:check` are clean.
   
   One thing this does not touch: `BinaryToDecimalUpdater` does not check the 
file's decimal scale against the Paimon type's the way 
`FixedLenByteArrayToDecimalUpdater` does, and `IntegerToDecimalUpdater` and 
`LongToDecimalUpdater` do not either. A mismatch there is read as a silently 
rescaled value. That is pre-existing across three updaters and adding the check 
would turn files that read today into hard failures, so it seems better as its 
own issue.
   


-- 
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