HPCC-35694 DFUQuery sort by compressed size by GordonSmith · Pull Request #20885 · hpcc-systems/HPCC-Platform

GordonSmith · 2026-01-23T13:06:49Z

Type of change:

This change is a bug fix (non-breaking change which fixes an issue).
This change is a new feature (non-breaking change which adds functionality).
This change improves the code (refactor or other change that does not change the functionality)
This change fixes warnings (the fix does not alter the functionality or the generated code)
This change is a breaking change (fix or feature that will cause existing behavior to change).
This change alters the query API (existing queries will have to be recompiled)

Checklist:

Smoketest:

Send notifications about my Pull Request position in Smoketest queue.
Test my draft Pull Request.

Testing:

github-actions · 2026-01-23T13:07:12Z

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-35694

Jirabot Action Result:
Assigning user: gordon.smith@lexisnexisrisk.com
Workflow Transition To: Merge Pending
Updated PR

Copilot

Pull request overview

This pull request adds sorting capability by compressed file size in the DFUQuery service. The change enables users to sort logical files by their compressed size, with appropriate fallback to uncompressed size for non-compressed files.

Changes:

Added frontend UI enhancements to display compressed size with visual indicators for compressed vs uncompressed files
Implemented backend sorting logic for compressed file size in the DFU service
Added mapping for compressed size field in the sort order configuration
Updated ESDL service interface documentation with version bumping guidelines

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
esp/src/src-react/components/Files.tsx	Changed CompressedFileSize column to be sortable, added dimmed styling for uncompressed files showing their size in brackets
esp/services/ws_dfu/ws_dfuService.hpp	Added declaration for findPositionByCompressedSize function
esp/services/ws_dfu/ws_dfuService.cpp	Implemented findPositionByCompressedSize sorting function and added CompressedFileSize to legacy sort mappings
dali/base/dadfs.cpp	Added logic to populate compressedsize field for file attributes, with fallback to origsize
.github/instructions/esp-service-interface.instructions.md	Added version bumping guidelines section to ESDL instructions

dali/base/dadfs.cpp

esp/services/ws_dfu/ws_dfuService.cpp

GordonSmith · 2026-01-23T13:13:38Z

@jakesmith tagging you for review as I think you made the changes in and around DFUSFsize, I tried to make this a minimal change, but IMO really should be refactored to:

size
compressedSize
fileSize = isCompressed ? compressedSize : size

esp/services/ws_dfu/ws_dfuService.cpp

dali/base/dadfs.cpp

jakesmith

@GordonSmith - had a looked into this/history of DFUQResultField::origsize etc. a bit.
See comment.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

esp/services/ws_dfu/ws_dfuService.cpp

dali/base/dadfs.cpp

esp/src/src-react/components/Files.tsx

esp/src/src/Utility.ts

esp/src/src-react/hooks/file.ts

esp/src/src-react/components/LogicalFileSummary.tsx

esp/src/src-react/components/Files.tsx

Signed-off-by: Gordon Smith<GordonJSmith@gmail.com>

GordonSmith · 2026-02-16T12:53:50Z

rebased

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

esp/src/src-react/hooks/file.ts

esp/src/src/Utility.ts

esp/src/src-react/components/IndexFileSummary.tsx

esp/src/src-react/components/Files.tsx

Copilot · 2026-02-16T12:59:42Z

dali/base/dadfs.cpp

+            if (options.includeField(DFUQResultField::origsize))
+            {
+                const char *propName = getDFUQResultFieldName(DFUQResultField::origsize);
+                attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));


This block sets @origsize to the value of @origsize, which is effectively a no-op when present, but will also create @origsize = -1 when absent. If the goal is to ensure the field exists only when already populated, consider guarding with hasProp(propName) (or omit this block entirely). If the goal is to force materialization, add a clarifying comment and avoid emitting -1 as a sentinel that could leak into clients.

Suggested change

attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));

if (attr->hasProp(propName))

attr->setPropInt64(propName, attr->getPropInt64(propName));

I kind of agree I suppose, but not quite what copilot is suggesting. The code is trying to default this prop to -1 if not set, so it would be better to do:

if (!attr->hasProp(propName)) attr->setPropInt64(propName, -1);

But, re. prev comments that were on lines 14206-14208..
and comment at end of line on 14210 "//Sort the files with empty size to front"
Aren't blank entries already at front?

.github/instructions/esp-service-interface.instructions.md

jakesmith

@GordonSmith - please see comments/questions.

jakesmith · 2026-02-20T16:18:40Z

dali/base/dadfs.cpp

+                // Size field is notionally the size on disk
                const char *propName = getDFUQResultFieldName(DFUQResultField::size);
-                attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));//Sort the files with empty size to front
+                attr->setPropInt64(propName, isCompressed(*attr) ? attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::compressedsize), -1) : attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));


DFUQResultField::size is a special field that only this sets (@DFUSFsize)

@GordonSmith - as far I could see last time I looked, Eclwatch (nor nothing else uses it).. I thought Eclwatch used "@SiZe" which is DFUQResultField::origsize .. ? (see comment in code here prior to this change).

If so this code should be deleted (and probably DFUQResultField::size/@DFUSFsize, but that could be part of a separate cleanup PR.

To elaborate on what Jake's saying here as best as I can see, the @DFUSFsize value is not exposed in the DFUQuery response. The legacyMappings are used to map name mismatches between the SortBy strings submitted in the request and the PTree attributes returned from CDistributedFileDirectory::getLogicalFiles (which uses deserializeFileAttrIter(...) if I'm correct to create the returned PTree).

The missing link is that the WsDFUHelpers::addToLogicalFileList(...) function isn't pulling out the @DFUSFsize attribute from the response. Even if it were, I'm not entirely sure where it should be returned.

Currently it pulls @size aka DFUQResultField::origsize to populate IntSize and TotalSize response fields. And it pulls @compressedSize to populate the CompressedFileSize response field.

The DFUQuery response would probably need to be modified with a new field for something like FileSize to hold @DFUSFsize if you wanted it:

https://github.com/hpcc-systems/HPCC-Platform/blob/master/esp/scm/ws_dfu_common.ecm

jakesmith · 2026-02-20T16:23:23Z

dali/base/dadfs.cpp

+            if (options.includeField(DFUQResultField::origsize))
+            {
+                const char *propName = getDFUQResultFieldName(DFUQResultField::origsize);
+                attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));


I kind of agree I suppose, but not quite what copilot is suggesting. The code is trying to default this prop to -1 if not set, so it would be better to do:

if (!attr->hasProp(propName)) attr->setPropInt64(propName, -1);

But, re. prev comments that were on lines 14206-14208..
and comment at end of line on 14210 "//Sort the files with empty size to front"
Aren't blank entries already at front?

jakesmith · 2026-02-20T16:24:34Z

testing/unittests/dalitests.cpp

            CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: Missing cost attributes", costAttrsPresent);
            bool dirAttrsPresent = attrs.hasProp("@directory");
            CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: directory attribute should NOT be present", !dirAttrsPresent);
+            CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: size field missing", attrs.hasProp("@DFUSFsize"));


I don't know why we have @DFUSFsize at the moment - what if anything uses it?

.github/instructions/esp-service-interface.instructions.md

esp/services/ws_dfu/ws_dfuService.cpp

asselitx · 2026-02-26T18:00:21Z

dali/base/dadfs.cpp

+                // Size field is notionally the size on disk
                const char *propName = getDFUQResultFieldName(DFUQResultField::size);
-                attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));//Sort the files with empty size to front
+                attr->setPropInt64(propName, isCompressed(*attr) ? attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::compressedsize), -1) : attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));


To elaborate on what Jake's saying here as best as I can see, the @DFUSFsize value is not exposed in the DFUQuery response. The legacyMappings are used to map name mismatches between the SortBy strings submitted in the request and the PTree attributes returned from CDistributedFileDirectory::getLogicalFiles (which uses deserializeFileAttrIter(...) if I'm correct to create the returned PTree).

The missing link is that the WsDFUHelpers::addToLogicalFileList(...) function isn't pulling out the @DFUSFsize attribute from the response. Even if it were, I'm not entirely sure where it should be returned.

Currently it pulls @size aka DFUQResultField::origsize to populate IntSize and TotalSize response fields. And it pulls @compressedSize to populate the CompressedFileSize response field.

The DFUQuery response would probably need to be modified with a new field for something like FileSize to hold @DFUSFsize if you wanted it:

https://github.com/hpcc-systems/HPCC-Platform/blob/master/esp/scm/ws_dfu_common.ecm

… order mappings

Copilot AI review requested due to automatic review settings January 23, 2026 13:06

Copilot started reviewing on behalf of GordonSmith January 23, 2026 13:07 View session

GordonSmith requested a review from jeclrsg January 23, 2026 13:11

Copilot AI reviewed Jan 23, 2026

View reviewed changes

dali/base/dadfs.cpp Outdated Show resolved Hide resolved

esp/services/ws_dfu/ws_dfuService.cpp Outdated Show resolved Hide resolved

GordonSmith requested a review from jakesmith January 23, 2026 13:11

GordonSmith force-pushed the HPCC-35694-DFUSORTBY_COMPRESSEDSIZE branch 2 times, most recently from 320b00c to c689e95 Compare January 23, 2026 14:22

GordonSmith commented Jan 23, 2026

View reviewed changes

esp/services/ws_dfu/ws_dfuService.cpp Outdated Show resolved Hide resolved

GordonSmith force-pushed the HPCC-35694-DFUSORTBY_COMPRESSEDSIZE branch from c689e95 to cd736df Compare January 23, 2026 14:37

jeclrsg reviewed Jan 23, 2026

View reviewed changes

esp/services/ws_dfu/ws_dfuService.cpp Show resolved Hide resolved

jakesmith reviewed Jan 27, 2026

View reviewed changes

dali/base/dadfs.cpp Outdated Show resolved Hide resolved

jakesmith reviewed Jan 27, 2026

View reviewed changes

GordonSmith marked this pull request as draft January 29, 2026 08:05

GordonSmith force-pushed the HPCC-35694-DFUSORTBY_COMPRESSEDSIZE branch 2 times, most recently from 72afcb2 to e38741a Compare February 9, 2026 12:47

GordonSmith requested a review from Copilot February 9, 2026 12:48

GordonSmith marked this pull request as ready for review February 9, 2026 12:49

Copilot started reviewing on behalf of GordonSmith February 9, 2026 12:49 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

GordonSmith force-pushed the HPCC-35694-DFUSORTBY_COMPRESSEDSIZE branch 2 times, most recently from 6296923 to e5d793e Compare February 12, 2026 09:04

GordonSmith added 2 commits February 16, 2026 12:50

HPCC-35694 Refactor file size + compression ratio

237b8fa

Signed-off-by: Gordon Smith<GordonJSmith@gmail.com>

HPCC-35694 Refactor file size + compression ratio

9b85d09

Signed-off-by: Gordon Smith<GordonJSmith@gmail.com>

GordonSmith force-pushed the HPCC-35694-DFUSORTBY_COMPRESSEDSIZE branch from e5d793e to 9b85d09 Compare February 16, 2026 12:53

GordonSmith requested review from Copilot, jakesmith and jeclrsg February 16, 2026 12:53

Copilot AI reviewed Feb 16, 2026

View reviewed changes

Copilot started reviewing on behalf of GordonSmith February 16, 2026 13:03 View session

Address latest copilot comments.

fe68908

jeclrsg approved these changes Feb 19, 2026

View reviewed changes

jakesmith reviewed Feb 20, 2026

View reviewed changes

GordonSmith requested a review from asselitx February 26, 2026 09:29

asselitx requested changes Feb 26, 2026

View reviewed changes

Refactor ESP service interface instructions and update DFU query sort…

342f4dc

… order mappings

	attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));
	if (attr->hasProp(propName))
	attr->setPropInt64(propName, attr->getPropInt64(propName));

Conversation

GordonSmith commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change:

Checklist:

Smoketest:

Testing:

Uh oh!

github-actions bot commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

GordonSmith commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakesmith left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GordonSmith commented Feb 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

jakesmith Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakesmith left a comment

Choose a reason for hiding this comment

Uh oh!

jakesmith Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

asselitx Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

jakesmith Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jakesmith Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

asselitx Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

GordonSmith commented Jan 23, 2026 •

edited

Loading

GordonSmith commented Jan 23, 2026 •

edited

Loading