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

HPCC-31496 Allow field translation that only removes fields - and does not add blank values #18496

Closed
wants to merge 1 commit into from
Closed

HPCC-31496 Allow field translation that only removes fields - and does not add blank values #18496

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
3 changes: 3 additions & 0 deletions common/thorhelper/thorcommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2148,6 +2148,9 @@ static bool getTranslators(Owned<const IDynamicTransform> &translator, Owned<con
if (!translator->canTranslate())
throw MakeStringException(0, "Untranslatable record layout mismatch detected for file %s", tracing);

if (mode == RecordTranslationMode::PayloadRemoveOnly && translator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", tracing);

if (translator->needsTranslate())
{
if (keyedTranslator && (sourceFormat != expectedFormat))
Expand Down
3 changes: 3 additions & 0 deletions common/thorhelper/thorread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ void DiskReadMapping::ensureTranslators() const
if (!translator->canTranslate())
throw MakeStringException(0, "Untranslatable record layout mismatch detected for file %s", filename);

if (mode == RecordTranslationMode::PayloadRemoveOnly && translator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", filename);

if (translator->needsTranslate())
{
if (sourceMeta != expectedMeta)
Expand Down
12 changes: 12 additions & 0 deletions ecl/hthor/hthorkey.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,8 @@ const IDynamicTransform * CHThorIndexReadActivityBase::getLayoutTranslator(IDist
Owned<const IDynamicTransform> payloadTranslator = createRecordTranslator(projectedFormat->queryRecordAccessor(true), actualFormat->queryRecordAccessor(true));
if (!payloadTranslator->canTranslate())
throw MakeStringException(0, "Untranslatable key layout mismatch reading index %s", f->queryLogicalName());
if (getLayoutTranslationMode() == RecordTranslationMode::PayloadRemoveOnly && payloadTranslator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", f->queryLogicalName());
if (payloadTranslator->needsTranslate())
return payloadTranslator.getClear();
return nullptr;
Expand All @@ -715,6 +717,8 @@ void CHThorIndexReadActivityBase::verifyIndex(IKeyIndex * idx)
{
if (!layoutTrans->canTranslate())
throw MakeStringException(0, "Untranslatable key layout mismatch reading index %s", df->queryLogicalName());
if (getLayoutTranslationMode() == RecordTranslationMode::PayloadRemoveOnly && layoutTrans->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", df->queryLogicalName());
}
else
{
Expand Down Expand Up @@ -2462,6 +2466,8 @@ class CHThorFlatFetchActivity : public CHThorFetchActivityBase, public IFlatFetc
translator->describe();
if (translator->canTranslate())
{
if (getLayoutTranslationMode()==RecordTranslationMode::PayloadRemoveOnly && translator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", f->queryLogicalName());
if (getLayoutTranslationMode()==RecordTranslationMode::None)
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled", f->queryLogicalName());
VStringBuffer msg("Record layout translation required for %s", f->queryLogicalName());
Expand Down Expand Up @@ -4102,6 +4108,8 @@ class CHThorKeyedJoinActivity : public CHThorThreadedActivityBase, implements I
throw MakeStringException(0, "Untranslatable key layout mismatch reading index %s", f->queryLogicalName());
if (payloadTranslator->keyedTranslated())
throw MakeStringException(0, "Untranslatable key layout mismatch reading index %s - keyed fields do not match", f->queryLogicalName());
if (getLayoutTranslationMode()==RecordTranslationMode::PayloadRemoveOnly && payloadTranslator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", f->queryLogicalName());
if (getLayoutTranslationMode()==RecordTranslationMode::None)
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled", f->queryLogicalName());
VStringBuffer msg("Record layout translation required for %s", f->queryLogicalName());
Expand All @@ -4119,6 +4127,8 @@ class CHThorKeyedJoinActivity : public CHThorThreadedActivityBase, implements I
{
if (!trans->canTranslate())
throw MakeStringException(0, "Untranslatable key layout mismatch reading index %s", f->queryLogicalName());
if (getLayoutTranslationMode() == RecordTranslationMode::PayloadRemoveOnly && trans->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", f->queryLogicalName());
}
else
{
Expand Down Expand Up @@ -4149,6 +4159,8 @@ class CHThorKeyedJoinActivity : public CHThorThreadedActivityBase, implements I
translator.setown(createRecordTranslator(helper.queryProjectedDiskRecordSize()->queryRecordAccessor(true), actualDiskMeta->queryRecordAccessor(true)));
if (translator->canTranslate())
{
if (getLayoutTranslationMode()==RecordTranslationMode::PayloadRemoveOnly && translator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", f->queryLogicalName());
if (getLayoutTranslationMode()==RecordTranslationMode::None)
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled", f->queryLogicalName());
VStringBuffer msg("Record layout translation required for %s", f->queryLogicalName());
Expand Down
4 changes: 2 additions & 2 deletions helm/hpcc/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1825,8 +1825,8 @@
"description": "Should the index node memory allocation flush the cache and retry if memory allocation fails"
},
"fieldTranslationEnabled": {
"default": "payload",
"enum": ["false", "true", "payload"],
"default": "payloadRemoveOnly",
"enum": ["false", "true", "payload", "payloadRemoveOnly"],
"description": "Enables translation (where possible) of mismatched file layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only"
},
"highTimeout": {
Expand Down
5 changes: 3 additions & 2 deletions initfiles/componentfiles/configschema/xsd/eclagent.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,14 @@
hpcc:presetValue="false"
hpcc:tooltip="eclAgent analyzes workunit post-execution"/>
<xs:attribute name="fieldTranslationEnabled" hpcc:displayName="Enable Field Translation"
hpcc:presetValue="payload"
hpcc:presetValue="payloadRemoveOnly"
hpcc:tooltip="Enables translation (where possible) of mismatched index layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only">
<xs:simpleType>
<xs:restriction base="xs:string">
<xs:enumeration value="false" hpcc:displayName="False" hpcc:desciption=""/>
<xs:enumeration value="true" hpcc:displayName="true" hpcc:description=""/>
<xs:enumeration value="true" hpcc:displayName="True" hpcc:description=""/>
<xs:enumeration value="payload" hpcc:displayName="Payload" hpcc:description=""/>
Copy link
Member

Choose a reason for hiding this comment

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

Enumeration needs to still include payload. Same for all other instances.

<xs:enumeration value="payloadRemoveOnly" hpcc:displayName="PayloadRemoveOnly" hpcc:description=""/>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
Expand Down
5 changes: 3 additions & 2 deletions initfiles/componentfiles/configschema/xsd/roxie.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,14 @@
hpcc:displayName="flushJHtreeCacheOnOOM" hpcc:presetValue="true"
hpcc:tooltip="Should the index node memory allocation flush the cache and retry if memory allocation fails"/>
<xs:attribute name="fieldTranslationEnabled" hpcc:displayName="Enable Field Translation"
hpcc:presetValue="payload"
hpcc:presetValue="payloadRemoveOnly"
hpcc:tooltip="Enables translation (where possible) of mismatched index layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only">
<xs:simpleType>
<xs:restriction base="xs:string">
<xs:enumeration value="false" hpcc:displayName="False" hpcc:desciption=""/>
<xs:enumeration value="true" hpcc:displayName="true" hpcc:description=""/>
<xs:enumeration value="true" hpcc:displayName="True" hpcc:description=""/>
<xs:enumeration value="payload" hpcc:displayName="Payload" hpcc:description=""/>
<xs:enumeration value="payloadRemoveOnly" hpcc:displayName="PayloadRemoveOnly" hpcc:description=""/>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
Expand Down
5 changes: 3 additions & 2 deletions initfiles/componentfiles/configschema/xsd/thor.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,14 @@
<xs:attribute name="httpGlobalIdHeader" type="xs:string" hpcc:displayName="HTTP GlobalId Header" hpcc:presetValue="HPCC-Global-Id"
hpcc:tooltip="HTTP Header field to use for sending and receiving GlobalId"/>
<xs:attribute name="fieldTranslationEnabled" hpcc:displayName="Enable Field Translation"
hpcc:presetValue="payload"
hpcc:presetValue="payloadRemoveOnly"
hpcc:tooltip="Enables translation (where possible) of mismatched index layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only">
<xs:simpleType>
<xs:restriction base="xs:string">
<xs:enumeration value="false" hpcc:displayName="False" hpcc:desciption=""/>
<xs:enumeration value="true" hpcc:displayName="true" hpcc:description=""/>
<xs:enumeration value="true" hpcc:displayName="True" hpcc:description=""/>
<xs:enumeration value="payload" hpcc:displayName="Payload" hpcc:description=""/>
<xs:enumeration value="payloadRemoveOnly" hpcc:displayName="PayloadRemoveOnly" hpcc:description=""/>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
Expand Down
3 changes: 2 additions & 1 deletion initfiles/componentfiles/configxml/eclagent_config.xsd.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@
</xs:annotation>
</xs:attribute>

<xs:attribute name="fieldTranslationEnabled" use="optional" default="payload">
<xs:attribute name="fieldTranslationEnabled" use="optional" default="payloadRemoveOnly">
<xs:annotation>
<xs:appinfo>
<tooltip>Enables translation (where possible) of mismatched file layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only</tooltip>
Expand All @@ -201,6 +201,7 @@
<xs:enumeration value="false"/>
<xs:enumeration value="true"/>
<xs:enumeration value="payload"/>
<xs:enumeration value="payloadRemoveOnly"/>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
Expand Down
3 changes: 2 additions & 1 deletion initfiles/componentfiles/configxml/roxie.xsd.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@
</xs:appinfo>
</xs:annotation>
</xs:attribute>
<xs:attribute name="fieldTranslationEnabled" use="optional" default="payload">
<xs:attribute name="fieldTranslationEnabled" use="optional" default="payloadRemoveOnly">
<xs:annotation>
<xs:appinfo>
<tooltip>Enables translation (where possible) of mismatched file layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only</tooltip>
Expand All @@ -649,6 +649,7 @@
<xs:enumeration value="false"/>
<xs:enumeration value="true"/>
<xs:enumeration value="payload"/>
<xs:enumeration value="payloadRemoveOnly"/>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
Expand Down
3 changes: 2 additions & 1 deletion initfiles/componentfiles/configxml/thor.xsd.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@
</xs:appinfo>
</xs:annotation>
</xs:attribute>
<xs:attribute name="fieldTranslationEnabled" use="optional" default="payload">
<xs:attribute name="fieldTranslationEnabled" use="optional" default="payloadRemoveOnly">
<xs:annotation>
<xs:appinfo>
<tooltip>Enables translation (where possible) of mismatched file layouts on-the-fly. Specify 'payload' to attempt to translate payload fields only</tooltip>
Expand All @@ -688,6 +688,7 @@
<xs:enumeration value="false"/>
<xs:enumeration value="true"/>
<xs:enumeration value="payload"/>
<xs:enumeration value="payloadRemoveOnly"/>
</xs:restriction>
</xs:simpleType>
</xs:attribute>
Expand Down
2 changes: 1 addition & 1 deletion initfiles/etc/DIR_NAME/environment.xml.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
enableKeyDiff="true"
enableSysLog="true"
fastLaneQueue="true"
fieldTranslationEnabled="payload"
fieldTranslationEnabled="payloadRemoveOnly"
Copy link
Member

@jakesmith jakesmith Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This should be consistent with testing/regress/environment.xml.in.

Copy link
Contributor Author

@jackdelv jackdelv Apr 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The changes between initfiles/etc/DIR_NAME/environment.xml.in and testing/regress/environment.xml.in look consistent to me. I'm probably misunderstanding what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

agreed, I missed the testing/regress change.

flushJHtreeCacheOnOOM="true"
forceStdLog="false"
heapRetainMemory="false"
Expand Down
2 changes: 2 additions & 0 deletions roxie/ccd/ccdfile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3013,6 +3013,8 @@ class CResolvedFile : implements IResolvedFileCreator, implements ISafeSDSSubscr
}
if (!translator || !translator->canTranslate())
throw MakeStringException(ROXIE_MISMATCH, "Untranslatable record layout mismatch detected for file %s", subname);
else if (mode == RecordTranslationMode::PayloadRemoveOnly && translator->hasNewFields())
throw MakeStringException(0, "Translatable file layout mismatch reading file %s but translation disabled when expected fields are missing from source.", subname);
else if (translator->needsTranslate())
{
if (fileMode==FileFormatMode::index && translator->keyedTranslated())
Expand Down
4 changes: 2 additions & 2 deletions roxie/ccd/ccdmain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ unsigned defaultTraceLimit = 10;
unsigned watchActivityId = 0;
unsigned testAgentFailure = 0;
RelaxedAtomic<unsigned> restarts;
RecordTranslationMode fieldTranslationEnabled = RecordTranslationMode::Payload;
RecordTranslationMode fieldTranslationEnabled = RecordTranslationMode::PayloadRemoveOnly;
bool mergeAgentStatistics = true;
PTreeReaderOptions defaultXmlReadFlags = ptr_ignoreWhiteSpace;
bool runOnce = false;
Expand Down Expand Up @@ -1181,7 +1181,7 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
agentQueryReleaseDelaySeconds = topology->getPropInt("@agentQueryReleaseDelaySeconds", topology->getPropInt("@slaveQueryReleaseDelaySeconds", 60)); // legacy name
coresPerQuery = topology->getPropInt("@coresPerQuery", 0);

fieldTranslationEnabled = RecordTranslationMode::Payload;
fieldTranslationEnabled = RecordTranslationMode::PayloadRemoveOnly;
const char *val = topology->queryProp("@fieldTranslationEnabled");
if (val)
fieldTranslationEnabled = getTranslationMode(val, false);
Expand Down
2 changes: 1 addition & 1 deletion roxie/ccd/ccdstate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2463,7 +2463,7 @@ class CRoxiePackageSetManager : implements IRoxieQueryPackageManagerSet, impleme
if (val)
fieldTranslationEnabled = getTranslationMode(val, false);
else
fieldTranslationEnabled = RecordTranslationMode::Payload;
fieldTranslationEnabled = RecordTranslationMode::PayloadRemoveOnly;
val = getTranslationModeText(fieldTranslationEnabled);
topology->setProp("@fieldTranslationEnabled", val);
}
Expand Down
13 changes: 12 additions & 1 deletion rtl/eclrtl/rtldynfield.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@

extern ECLRTL_API RecordTranslationMode getTranslationMode(const char *val, bool isLocal)
{
if (isEmptyString(val) || strToBool(val) || strieq(val, "payload"))
if (isEmptyString(val) || strToBool(val) || strieq(val, "payloadRemoveOnly"))
return RecordTranslationMode::PayloadRemoveOnly;
else if (strieq(val, "payload"))
return RecordTranslationMode::Payload;
else if (strieq(val, "alwaysDisk") || strieq(val, "disk"))
{
Expand All @@ -60,6 +62,7 @@ extern ECLRTL_API const char *getTranslationModeText(RecordTranslationMode val)
case RecordTranslationMode::AlwaysDisk: return "alwaysDisk";
case RecordTranslationMode::AlwaysECL: return "alwaysECL";
case RecordTranslationMode::Payload: return "payload";
case RecordTranslationMode::PayloadRemoveOnly: return "payloadRemoveOnly";
case RecordTranslationMode::None: return "off";
}
throwUnexpected();
Expand Down Expand Up @@ -1085,6 +1088,10 @@ class GeneralRecordTranslator : public CInterfaceOf<IDynamicTransform>
{
return (matchFlags & match_keychange) != 0;
}
virtual bool hasNewFields() const override
{
return (matchFlags & match_none) != 0;
}
private:
void doDescribe(unsigned indent) const
{
Expand Down Expand Up @@ -1948,6 +1955,10 @@ class CloneVirtualRecordTranslator : public CInterfaceOf<IDynamicTransform>
{
return false;
}
virtual bool hasNewFields() const override
{
return false;
}
private:
void doDescribe(unsigned indent) const
{
Expand Down
4 changes: 3 additions & 1 deletion rtl/eclrtl/rtldynfield.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ enum class RecordTranslationMode : byte
Payload = 2, // Translate all fields in datasets, and only payload fields in indexes
AlwaysDisk = 3, // Always translate - even if wouldn't normally (e.g. csv/xml source read as binary), or crcs happen to match
AlwaysECL = 4, // Ignore the published format - can make sense to force no translation e.g. when field names have changed
Unspecified = 5
PayloadRemoveOnly = 5, // Allow fields to be removed from the incoming dataset, but not allow fields to be missing
Unspecified = 6
}; // AlwaysDisk and AlwaysECL are for testing purposes only, and can only be set per file (not globally)

extern ECLRTL_API RecordTranslationMode getTranslationMode(const char *modeStr, bool isLocal);
Expand Down Expand Up @@ -139,6 +140,7 @@ interface IDynamicTransform : public IInterface
virtual bool needsTranslate() const = 0;
virtual bool keyedTranslated() const = 0;
virtual bool needsNonVirtualTranslate() const = 0;
virtual bool hasNewFields() const = 0;
};

interface IKeyTranslator : public IInterface
Expand Down
2 changes: 1 addition & 1 deletion testing/regress/ecl/layouttrans.ecl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ version := #IFDEFINED(root.version, 1);

//--- end of version configuration ---

#option ('layoutTranslation', true);
#option ('layoutTranslation', 'payload');
#onwarning (4515, ignore);
#onwarning (4522, ignore);
#onwarning (4523, ignore);
Expand Down
2 changes: 2 additions & 0 deletions testing/regress/ecl/sqtrans.ecl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ multiPart := #IFDEFINED(root.multiPart, false);

//--- end of version configuration ---

#option('layoutTranslation', 'payload');

//check that compound disk read activities workcorrectly when the input file is translated
//include keyed filters to ensure they are also translated correctly

Expand Down
2 changes: 1 addition & 1 deletion testing/regress/ecl/translatedisk.ecl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ optRemoteRead := #IFDEFINED(root.optRemoteRead, true);

#onwarning(2036, ignore);
#onwarning(4522, ignore);
#option ('layoutTranslation', true);
#option ('layoutTranslation', 'payload');
#option('forceRemoteRead', optRemoteRead);
import $.Setup;

Expand Down
2 changes: 1 addition & 1 deletion testing/regress/environment.xml.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
enableKeyDiff="true"
enableSysLog="true"
fastLaneQueue="true"
fieldTranslationEnabled="payload"
fieldTranslationEnabled="payloadRemoveOnly"
flushJHtreeCacheOnOOM="true"
forceStdLog="false"
heapRetainMemory="false"
Expand Down
Loading
Loading