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.

Sign up for GitHub

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

HPCC4J-562 Odd unsigned decimals incorrect scale #666

Merged
merged 1 commit into from
Dec 13, 2023
Merged

HPCC4J-562 Odd unsigned decimals incorrect scale #666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion dfsclient/src/main/java/org/hpccsystems/dfs/client/BinaryRecordReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,15 @@ private BigDecimal getUnsignedDecimal(int numDigits, int precision, int dataLen)
BigDecimal ret = new BigDecimal(0);

int idx = 0;
int curDigit = numDigits - 1;
int curDigit = numDigits;

// If the # of digits is odd the top most nibble is unused and we don't want to include it
// in the scale calculations below. Due to how the scale calculation works below this means
// we decrement the starting value of curDigit in the case of even length decimals
if ((numDigits % 2) == 0)
Copy link
Member

Choose a reason for hiding this comment

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

It's not entirely obvious to me why this logic is needed, perhaps an inline comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, added a comment and squashed

{
curDigit--;
}

while (idx < dataLen)
{
Expand Down
7 changes: 6 additions & 1 deletion dfsclient/src/test/resources/generate-datasets.ecl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ childRec := {STRING8 childField1, INTEGER8 childField2, REAL8 childField3};
rec := {INTEGER8 int8, UNSIGNED8 uint8, INTEGER4 int4, UNSIGNED4 uint4,
INTEGER2 int2, UNSIGNED2 uint2,
REAL8 r8, REAL4 r4,
DECIMAL16_8 dec16, UDECIMAL16_8 udec16,
DECIMAL16_8 dec16,
DECIMAL15_8 dec15,
UDECIMAL16_8 udec16,
UDECIMAL15_8 udec15,
QSTRING qStr,
STRING8 fixStr8,
STRING str,
Expand All @@ -33,7 +36,9 @@ ds := DATASET(totalrecs1, transform(rec,
self.r8 := (REAL)(random() % unique_values);
self.r4 := (REAL)(random() % unique_values);
self.dec16 := (REAL)(random() % unique_values);
self.dec15 := (REAL)(random() % unique_values);
self.udec16 := (REAL)(random() % unique_values);
self.udec15 := (REAL)(random() % unique_values);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a corresponding test to verify these odd len decimals are correctly read?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rpastrana Yes, tests in DFSReadWriteTest read those generated datasets, write them back, and then compare the datasets against the generated datasets for correctness.

self.qStr := (STRING)(random() % unique_values);
self.fixStr8 := (STRING)(random() % unique_values);
self.str := (STRING)(random() % unique_values);
Expand Down
Loading