nzw921rx commented on PR #11415:
URL: https://github.com/apache/seatunnel/pull/11415#issuecomment-4951933153
> Thanks for working on this. I checked the latest implementation and the
reported String -> Integer / BigDecimal -> Long failures.
>
> Allowing the serializer to preserve the runtime representation of numeric
values looks useful. However, I think the fallback for non-numeric values is
currently too broad and may turn a visible type error into silent data
corruption.
>
> Non-numeric fallback
>
> createNumericNode(...) currently ends with:
>
> `return mapper.getNodeFactory().textNode(String.valueOf(value));`
>
> This works for the reported String value, but it also accepts every other
Java object.
>
> For example:
>
> ```text
> byte[] -> "[B@..."
> Object[] -> "[Ljava.lang.Object;@..."
> Map -> "{key=value}"
> ```
>
> These values are not reliable JSON representations of the original data.
In a heterogeneous multi-table job, a binary, array, map, or nested value could
therefore be serialized successfully but with corrupted content.
>
> Could we please narrow this behavior?
>
> One possible approach would be:
>
> * preserve the runtime representation for supported Number implementations;
> * explicitly support CharSequence as JSON text if that is part of the
intended heterogeneous contract;
> * fail clearly for other unsupported runtime types rather than applying
String.valueOf() to every object.
>
> Alternatively, if arbitrary heterogeneous values should be supported, they
should be converted structurally instead of being flattened through toString().
>
> Schema contract
>
> The new test uses an INT field containing a String and a LONG field
containing BigDecimal("123.45").
>
> This proves that serialization no longer fails, but it also means that the
generated JSON no longer conforms to the declared SeaTunnelRowType. Reading the
same JSON with that schema would either fail or coerce the value and lose
information.
>
> Because JsonSerializationSchema is a common formatter used by RabbitMQ,
Kafka, file sinks, and other connectors, this becomes a global behavior change
rather than a RabbitMQ-specific fix.
>
> Could we either:
>
> 1. keep the relaxed heterogeneous behavior scoped to the affected
multi-table/schemaless path; or
> 2. document it as an intentional common JSON contract and add broader
compatibility tests?
>
> Suggested tests
>
> It would be helpful to cover:
>
> * every supported runtime numeric representation;
> * a String value under a numeric catalog field;
> * unsupported non-numeric values such as byte[], arrays, or maps;
> * ideally the actual multi-table CDC to RabbitMQ regression path.
>
> Issue scope
>
> #10621 also mentions the required queue_name and fanout/multi-queue
behavior, which this PR does not appear to change. It may be better to use
Partially addresses #10621, or create a follow-up issue before closing it.
>
> The numeric ClassCastException direction looks good. I would only ask that
the non-numeric fallback be made safer before merge so that invalid values do
not become silently corrupted JSON.
Withdraw！
--
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]
