Skip to content

Commit

Permalink
Fixed bug CORE-5847 : "Malformed string" instead of key value in PK v…
Browse files Browse the repository at this point in the history
…iolation error message
  • Loading branch information
hvlad committed Jun 19, 2018
1 parent e5dc6ef commit b9da7ba
Showing 1 changed file with 11 additions and 6 deletions.
17 changes: 11 additions & 6 deletions src/jrd/mov.cpp
Expand Up @@ -504,9 +504,9 @@ DescPrinter::DescPrinter(thread_db* tdbb, const dsc* desc, int mLen)

fb_assert(!desc->isBlob());

value = MOV_make_string2(tdbb, desc, ttype_dynamic);
const bool octets = (desc->isText() && desc->getTextType() == ttype_binary);

This comment has been minimized.

Copy link
@dyemanov

dyemanov Jun 19, 2018

Member

Could we please leave "octets" up to SQL users and use "binary" instead in the codebase? This wording looks more obvious / better understandable to me.

This comment has been minimized.

Copy link
@hvlad

hvlad Jun 19, 2018

Author Member

Would you like to change it in all 3 branches, or we could leave it as is, in this case ? ;)

This comment has been minimized.

Copy link
@dyemanov

dyemanov Jun 19, 2018

Member

master is enough ;-)

value = MOV_make_string2(tdbb, desc, octets ? ttype_binary : ttype_dynamic);

const int len = (int) value.length();
const char* const str = value.c_str();

if (desc->isText() || desc->isDateTime())
Expand All @@ -517,24 +517,29 @@ DescPrinter::DescPrinter(thread_db* tdbb, const dsc* desc, int mLen)
value.rtrim(pad);
}

if (desc->isText() && desc->getTextType() == ttype_binary)
if (octets)
{
Firebird::string hex;

int len = (int) value.length();
const bool cut = (len > (maxLen - 3) / 2);

This comment has been minimized.

Copy link
@asfernandes

asfernandes Aug 5, 2018

Member

@hvlad please put a comment explaining this code and its magic constants (5, 3, 2).

This comment has been minimized.

Copy link
@hvlad

hvlad Aug 6, 2018

Author Member

Done

if (cut)
len = (maxLen - 5) / 2;

char* s = hex.getBuffer(2 * len);

for (int i = 0; i < len; i++)
{
sprintf(s, "%02X", (int)(unsigned char) str[i]);
s += 2;
}

value = "x'" + hex + "'";
value = "x'" + hex + (cut ? "..." : "'");

This comment has been minimized.

Copy link
@dyemanov

dyemanov Jun 19, 2018

Member

Wouldn't it be better to print truncated values as 'ABC...' rather than 'ABC... (without final quote)? Consider compound keys: ("FIELD1" = 'ABC..., "FIELD2" = 'DEF') - this doesn't look very consistent with broken sequence of quotes.

This comment has been minimized.

Copy link
@hvlad

hvlad Jun 19, 2018

Author Member

It was my initial idea but later i found that code below work this way and decided to not change it

}
else
value = "'" + value + "'";
}

if (value.length() > maxLen)
if (value.length() > (FB_SIZE_T) maxLen)

This comment has been minimized.

Copy link
@dyemanov

dyemanov Jun 19, 2018

Member

I would consider making both maxLen and len unsigned to avoid these casts.

This comment has been minimized.

Copy link
@hvlad

hvlad Jun 19, 2018

Author Member

I also think it is better. Just didn't want to make more changes than necessary.
Again, would you like me to make this change in all 3 branches ? ;)

{
fb_assert(desc->isText());

Expand Down

0 comments on commit b9da7ba

Please sign in to comment.