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-11151 Support xpath(‘<>’) for write #10348
Conversation
When #OPTION('writeInlineContent', true); is used writing a field with XPATH('Name/<>') will output the string without XML/JSON encoding the content. This allows users to insert arbitrary XML/JSON into the output. Also works with sets of strings XPATH('SetName/Name/<>'). There are 3 forms: '<>' Writes the content without a root tag. Reading reads all content of parent. 'Name<>' Writes the content without a <Name> tag. Reading reads the content of the tag <Name>. 'Name/<>' Writes the content inside <Name>. Reading reads the content of the tag <Name>. Also fixes generated schemas to indicate locations where arbitrary content might appear. Signed-off-by: Anthony Fishbeck <anthony.fishbeck@lexisnexis.com>
fe6fce8
to
66eeb6c
Compare
Automated Smoketest: ✅
Install hpccsystems-platform-community_6.5.0-trunk0.el7.x86_64.rpm Unit tests result:
Regression test result:
HPCC Stop: OK |
@richardkchapman you may want to take a look at where this overlapped with your dynamic typeinfo changes. |
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.
@afishbeck some initial comments. It is probably worth me reviewing it all again later because it is quite a lot to take in at once.
Main issues are utf8 encoding of strings, and main comment is using enumerations instead of byte.
s.append("><xs:annotation><xs:appinfo hpcc:keyed=\"true\"/></xs:annotation></xs:any>\n"); | ||
else | ||
s.append("/>\n"); | ||
|
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.
trivial: extra newline
@@ -302,8 +302,8 @@ typedef IArrayOf<ITypeInfo> TypeInfoArray; | |||
interface ISchemaBuilder | |||
{ | |||
public: | |||
virtual void addField(const char * name, ITypeInfo & type, bool keyed) = 0; | |||
virtual void addSetField(const char * name, const char * itemname, ITypeInfo & type) = 0; | |||
virtual void addField(const char * name, ITypeInfo & type, bool keyed, byte contentFlags) = 0; |
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.
style: Cleaner to pass a named enumeration instead of a byte.
{ | ||
XPathContentInline = 0x01, | ||
XPathContentNamed = 0x02, | ||
XPathContentMixed = 0x04 |
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.
I don't think this is used. Could you reuse the previous enumeration, rather than defining a new one?
break; | ||
case type_unicode: | ||
len = getLength(type, cur); | ||
writer.outputUnicode(len, (UChar const *)cur, name); | ||
//tbd |
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.
MORE: Is this going to be implemented?
@@ -1607,7 +1628,10 @@ void CResultSetCursor::writeXmlText(IXmlWriter &writer, int columnIndex, const c | |||
case type_qstring: | |||
len = getLength(type, cur); | |||
rtlQStrToStrX(resultLen, resultStr, len, (const char *)cur); | |||
writer.outputString(resultLen, resultStr, name); | |||
if (meta.allowInlineContent && (contentFlags & XSBLD_contentInline)) | |||
writer.outputInline(resultLen, resultStr, (contentFlags & XSBLD_contentNamed) ? name : nullptr); |
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.
Although this is output inline it will still need to be converted from iso-8859-1 codepage to utf8.
To test ensure the strings contain some latin accented characters.
@@ -10923,7 +10930,7 @@ void HqlCppTranslator::buildXmlSerializeSetValues(BuildCtx & ctx, IHqlExpression | |||
CHqlBoundExpr boundCurElement; | |||
cursor->buildIterateLoop(loopctx, boundCurElement, false); | |||
OwnedHqlExpr curElement = boundCurElement.getTranslatedExpr(); | |||
buildXmlSerializeScalar(loopctx, curElement, itemName); | |||
buildXmlSerializeScalar(loopctx, curElement, itemName, false); |
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.
The fourth parameter is a contentFlag, not a boolean. Using an enum for the parameter would have caught it.
@@ -117,6 +117,26 @@ void CommonXmlWriter::outputString(unsigned len, const char *field, const char * | |||
} | |||
} | |||
|
|||
void CommonXmlWriter::outputInline(unsigned len, const char *field, const char *fieldname) |
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.
minor style: Cleaner if length parameter are size32_t rather than unsigned.
@@ -3277,7 +3277,7 @@ bool CWsWorkunitsEx::onWUResult(IEspContext &context, IEspWUResultRequest &req, | |||
const char* resultName = req.getResultName(); | |||
|
|||
Owned<DataCacheElement> data; | |||
if (!req.getBypassCachedResult()) | |||
if (0)//!req.getBypassCachedResult()) |
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.
more: Is this left in by mistake?
@@ -679,11 +679,17 @@ size32_t RtlStringTypeInfo::toXML(const byte * self, const byte * selfrow, const | |||
unsigned lenAscii; | |||
rtlDataAttr ascii; | |||
rtlEStrToStrX(lenAscii, ascii.refstr(), thisLength, str); | |||
target.outputString(lenAscii, ascii.getstr(), queryScalarXPath(field)); | |||
if (field->hasInlineContentXpath()) | |||
target.outputInline(lenAscii, ascii.getstr(), field->hasNamedContentXpath() ? queryScalarXPath(field) : nullptr); |
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.
Same issues of conversion to utf8.
target.outputUtf8(thisLength, str, queryScalarXPath(field)); | ||
if (field->hasInlineContentXpath()) | ||
{ | ||
target.outputInline(thisLength, str, field->hasNamedContentXpath() ? queryScalarXPath(field) : nullptr); |
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.
Need to be careful about length v size. (Worth naming size variables appropriately.)
@afishbeck This seems to be stalled (and needs rebasing) |
Will reopen when I return from PTO. |
When #OPTION('writeInlineContent', true); is used writing a field with
XPATH('Name/<>') will output the string without XML/JSON encoding
the content. This allows users to insert arbitrary XML/JSON into
the output.
Also works with sets of strings XPATH('SetName/Name/<>').
There are 3 forms:
'<>' Writes the content without a root tag. Reading reads all
content of parent.
'Name<>' Writes the content without a tag. Reading reads the
content of the tag .
'Name/<>' Writes the content inside . Reading reads the
content of the tag .
Also fixes generated schemas to indicate locations where arbitrary
content might appear.
Signed-off-by: Anthony Fishbeck anthony.fishbeck@lexisnexis.com
Type of change:
Checklist:
Testing: