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
HPCC-20090 Return partition/bloom information in WsDfu.DFUInfo #11420
Conversation
Signed-off-by: wangkx <kevin.wang@lexisnexis.com>
https://track.hpccsystems.com/browse/HPCC-20090 |
@afishbeck please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wangkx looks good, a couple questions.
esp/scm/ws_dfu.ecm
Outdated
@@ -160,6 +174,8 @@ ESPStruct [nil_remove] DFUFileDetail | |||
[min_ver("1.37"), json_inline(1)] string jsonInfo; | |||
[min_ver("1.37")] binary binInfo; | |||
[min_ver("1.38")] string PackageID; | |||
[min_ver("1.39")] ESPstruct DFUFilePartition Partitions; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this isn't an array should it be Partition instead of Partitions?
return; | ||
|
||
fieldCount++; | ||
if (fieldMask % 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is the most efficient way, but probably doesn't make much difference.
Signed-off-by: wangkx <kevin.wang@lexisnexis.com>
esp/scm/ws_dfu.ecm
Outdated
@@ -174,8 +174,8 @@ ESPStruct [nil_remove] DFUFileDetail | |||
[min_ver("1.37"), json_inline(1)] string jsonInfo; | |||
[min_ver("1.37")] binary binInfo; | |||
[min_ver("1.38")] string PackageID; | |||
[min_ver("1.39")] ESPstruct DFUFilePartition Partitions; | |||
[min_ver("1.39")] ESPstruct DFUFileBloom Blooms; | |||
[min_ver("1.39")] ESPstruct DFUFilePartition Partition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can only be one PARTITION attribute, but there can be several BLOOM attributes. I'm not sure the code below handles this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardkchapman for testing, I am trying to create an index with multiple BLOOM attributes using the following ECL code. The eclagent throws an exception: 'Published record size 22 for file thor::fred, does not match coded record size 13'. Please advise how to fix the ECL code.
rec := RECORD
string1 a;
string3 b;
string1 c;
unsigned d;
END;
boolean useBloom := true : stored('useBloom');
d := dataset('fred', rec, FLAT);
i1 := index(d, {a,b,c,d}, 'kw_ix_bloom_a_b_1');
buildindex(i1, overwrite, BLOOM(a, LIMIT(10), PROBABILITY(0.1)), BLOOM(b, LIMIT(12), PROBABILITY(0.2)));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind. The problem is in my data file. I fixed it and just creates an index with multiple BLOOM attributes. @wangkx
esp/scm/ws_dfu.ecm
Outdated
@@ -160,6 +174,8 @@ ESPStruct [nil_remove] DFUFileDetail | |||
[min_ver("1.37"), json_inline(1)] string jsonInfo; | |||
[min_ver("1.37")] binary binInfo; | |||
[min_ver("1.38")] string PackageID; | |||
[min_ver("1.39")] ESPstruct DFUFilePartition Partition; | |||
[min_ver("1.39")] ESPstruct DFUFileBloom Bloom; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can only be one PARTITION on an index but there can be several BLOOM attributes.
Signed-off-by: wangkx <kevin.wang@lexisnexis.com>
Automated Smoketest: ✅ Unit tests result:
Regression test result:
HPCC Stop: OK
|
@richardkchapman @afishbeck please review my last commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wangkx looks good.
Signed-off-by: wangkx kevin.wang@lexisnexis.com
Type of change:
Checklist:
Testing: