Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comma as a valid separator for numeric values #241

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

tr11
Copy link
Collaborator

@tr11 tr11 commented Feb 2, 2020

This PR adds support for numeric values with comma as the decimal separator.

Fixes issue #240.

Copy link
Collaborator

@yruslan yruslan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for your contribution!

@yruslan
Copy link
Collaborator

yruslan commented Feb 3, 2020

@hamc17 , it seems CI does not trigger for PRs created by external contributors.

@yruslan yruslan merged commit 57a46dd into AbsaOSS:master Feb 3, 2020
@a4032
Copy link

a4032 commented Feb 3, 2020

HI, the comma is indeed the most used decimal separator in west-europe. So If possible to fix this urgently? This is a blocking issue for us. Can you tell me when this will be available?
If not possible, we could not use cobrix maven library in our databricks solution.
thanks

@yruslan
Copy link
Collaborator

yruslan commented Feb 3, 2020

Hi, @a4032 ,
It is released as 2.0.2 already. It may take a couple of more hours to appear on Maven Central though,

@a4032
Copy link

a4032 commented Feb 3, 2020

unsuccessfully:

picture clause = PIC +9999999,99 USAGE DISPLAY.

installed library (maven) : za.co.absa.cobrix:spark-cobol_2.11:2.0.2

data = +0000100,00

after reading the file: the result = 'null'

when I use PIC X(11) it shows the correct value but is not numeric: '+0000100,00'

can you verify again please ?
thanks

@a4032
Copy link

a4032 commented Feb 3, 2020 via email

@yruslan
Copy link
Collaborator

yruslan commented Feb 3, 2020

I will take a look.

@yruslan
Copy link
Collaborator

yruslan commented Feb 3, 2020

The fix for the comma support has missed a fix for the decoder.
Currently, the fix is available in 2.0.3-SNAPSHOT. It will be in 2.0.3 after the fix is covered with a regression test.

@a4032
Copy link

a4032 commented Feb 3, 2020 via email

@tr11
Copy link
Collaborator Author

tr11 commented Feb 4, 2020

The fix for the comma support has missed a fix for the decoder.
Currently, the fix is available in 2.0.3-SNAPSHOT. It will be in 2.0.3 after the fix is covered with a regression test.

Oops, didn't even remember to check the decoder side. My bad, sorry!

@yruslan
Copy link
Collaborator

yruslan commented Feb 4, 2020

No worries. I should have checked it more thoroughly, but I'm ooo currently.

Thanks for the fix to the parser. It would have taken me much longer to figure it out.

@yruslan
Copy link
Collaborator

yruslan commented Feb 6, 2020

@a4032 , this should be fixed in 2.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants