tmgodinho commented on code in PR #7639:
URL: https://github.com/apache/ignite-3/pull/7639#discussion_r2833016062
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/row/RowAssembler.java:
##########
@@ -361,11 +360,6 @@ public RowAssembler appendDecimalNotNull(BigDecimal val)
throws SchemaMismatchEx
DecimalNativeType type = (DecimalNativeType) col.type();
- BigDecimal scaled = val.setScale(type.scale(), RoundingMode.HALF_UP);
Review Comment:
Good question. Yes. The actual tests that are checking for the exceptions
are:
https://github.com/apache/ignite-3/blob/084af6c3343e43d1d91de1eb5ad53ce08e2c2a57/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientMarshallingTest.java#L354-L383
Essentially, before the PR, the validation for decimals was done here. After
the PR, the validation is now done on Column#validate() with seemed to me more
similar to the other checks. For instance:
https://github.com/apache/ignite-3/blob/084af6c3343e43d1d91de1eb5ad53ce08e2c2a57/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientMarshallingTest.java#L340-L352
I also did not find much validation code on row assembler besides
`checkNull` and `checkType` and this decimals stuff. That's why I removed the
check here.
The only smell I found is that the validation itself is now done under the
gatherStatistics
(https://github.com/apache/ignite-3/blob/8b8d67c6710847614443d29e2655cdfb605e300b/modules/table/src/main/java/org/apache/ignite/internal/schema/marshaller/TupleMarshallerImpl.java#L226-L248)
method. This definitely flexes a bit for whoever is interested in finding the
validation method. Nonetheless, it was already that way before.
--
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]
