Fix to Handle Big Decimal Data Type

[priyankaku]

No description provided.

[dalelane]

Please restrict pull requests to addressing a single issue

[priyankaku]

Please restrict pull requests to addressing a single issue

Hi @dalelane , sorry it seems it got auto corrected by the IDE while formatting.
I will revert it back , PR was intended only to fix BigDecimal issue.

[priyankaku]

@dalelane , i have revert back the changes. Pushed only the required changes.

[dalelane]

We talked last week about how the converter currently doesn't support logical types - and how your work with the database connector has raised this as a new requirement.

A quick look at the Javadoc suggests that the four logical types in org.apache.kafka.connect.data are Date, Decimal, Time, Timestamp

Are you planning to add other logical types in follow-on pull requests? Or do you think there is a reason we should add support for only one logical type, but not any others?

[dalelane]

Under what situations could bytes be null?

[priyankaku]

Since that condition was already existing just kept as it is. I have not tested out the scenarios where it could be null.

[priyankaku]

We talked last week about how the converter currently doesn't support logical types - and how your work with the database connector has raised this as a new requirement.

A quick look at the Javadoc suggests that the four logical types in org.apache.kafka.connect.data are Date, Decimal, Time, Timestamp

Are you planning to add other logical types in follow-on pull requests? Or do you think there is a reason we should add support for only one logical type, but not any others?

Yes , I will be creating a new PR for addressing each logical types.

[dalelane]

Unless I'm reading this incorrectly, I don't think this if check could ever pass

Reading through this code, I can see we're inside the else branch of the if (Decimal.LOGICAL_NAME.equals(fieldSchema.name())) check from line 241

That means this if check could never be satisfied, and that the line below is dead and unreachable code

[priyankaku]

Sorry , missed to remove this block

[dalelane]

These two lines are overly complex and I think a little bit circular.

The aim of this code is to start with a BigDecimal object (source.get(field.name())) and turn it into a String to add to the XML element.

The code here is turning the BigDecimal into a String in 5 steps:

Step 1 - wrap the BigDecimal in String.valueOf

String.valueOf(theBigDecimal)

Step 2 - put the result in a constructor to turn it back into a BigDecimal

new BigDecimal(String.valueOf(theBigDecimal))

At this point, I think the code has done a round-trip and ended up with the value it started with. And it continues.

Step 3 - use a Connect helper method to turn a BigDecimal into a byte array

byte[] bytes = Decimal.fromLogical(schema, 
                                   new BigDecimal(String.valueOf(theBigDecimal)));

Step 4 - use the inverse Connect helper method to turn a byte array back into a BigDecimal

Decimal.toLogical(schema, bytes)

At this point, I think the code has now done another round-trip and ended up with the BigDecimal value it started with again.

Step 5 - use toString() to get a string version

Decimal.toLogical(schema, bytes).toString()

In summary... I think this code would produce an identical result if it was simplified to skip steps 1-4 and just do step 5:

strRepresentation = source.get(field.name()).toString();

[priyankaku]

Thank you for the suggestion , updated accordingly

[dalelane]

I think this is losing information.

We started with a Connect schema that specified a scale of 3 (three fraction digits). ref

But the schema here doesn't mention that.

We should try to ensure that the Connect schema is fully reflected in the XML schema.

Suggested change

	<xs:element type="xs:decimal" name="Price"/>
	<xs:element name="Price">
	<xs:simpleType>
	<xs:restriction base="xs:decimal">
	<xs:fractionDigits value="3"/>
	</xs:restriction>
	</xs:simpleType>
	</xs:element>

ref: https://learning.oreilly.com/library/view/xml-schema/0596002521/re24.html

[priyankaku]

updated

[Joel-hanson]

@dalelane could you please do a quick review for the latest changes.

[Joel-hanson]

This PR needs improvement by addressing all the combinations.