Skip to content
Permalink
Browse files
[ASTERIXDB-2949][RUN][FUN] SUBSTR function produces malformed string
- user model changes: no
- storage format changes: no
- interface changes: no

Details:
- Fix UTF8StringBuilder grow logic

UTF8StringBuilder initially takes an estimated length of the
string to be written and reserves space at the beginning
of the buffer to later store the length of the data written.
When the actual data written happens to be greater than the
estimated length requiring more space to store the length,
the string content needs to be shifted.

This patch is to fix the starting offset of the data to be shifted.
Also, the estimated length calculation of the substring method of
the UTF8StringPointable is modified to account for
SUBSTR(input_string, 0, num_chars_to_substring) with start offset = 0.

(cherry picked from commit cc6143b)

Change-Id: If36253ff884a9c19eaa130c4e5e926f2dd9eea1d
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/14264
Reviewed-by: Ali Alsuliman <ali.al.solaiman@gmail.com>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
  • Loading branch information
AliSolaiman authored and mblow committed Dec 1, 2021
1 parent f87564b commit 57014087d7883a211245ba8fac411ef8dfff5f69
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 3 deletions.
@@ -0,0 +1,25 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
/*
* Description : Test the issue described in ASTERIXDB-2949
* Success : Yes
*/


SELECT SUBSTR("•\tABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789\n•\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ\tABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789", 0, 1000) AS s;
@@ -0,0 +1 @@
{ "s": "•\tABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789\n•\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ\tABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789" }
@@ -9295,6 +9295,11 @@
<output-dir compare="Text">substr06</output-dir>
</compilation-unit>
</test-case>
<test-case FilePath="string">
<compilation-unit name="substr-ASTERIXDB-2949">
<output-dir compare="Text">substr-ASTERIXDB-2949</output-dir>
</compilation-unit>
</test-case>
<test-case FilePath="string">
<compilation-unit name="substring-after-1">
<output-dir compare="Text">substring-after-1</output-dir>
@@ -368,7 +368,9 @@ public static boolean substr(UTF8StringPointable src, int charOffset, int charLe
return false;
}

builder.reset(out, Math.min(utfLen - byteIdx, (int) (charLength * 1.0 * byteIdx / chIdx)));
// for byteIdx = 0, this estimate assumes that every char size = 1 byte
int estimateOutBytes = byteIdx == 0 ? charLength : (int) (charLength * 1.0 * byteIdx / chIdx);
builder.reset(out, Math.min(utfLen - byteIdx, estimateOutBytes));
chIdx = 0;
while (byteIdx < utfLen && chIdx < charLength) {
builder.appendChar(src.charAt(src.getMetaDataLength() + byteIdx));
@@ -85,8 +85,9 @@ public void finish() throws IOException {
for (int i = 0; i < diff; i++) {
out.writeByte(0);
}
for (int i = ary.getLength() - 1; i >= actualDataStart + diff; i--) {
ary.getByteArray()[i] = ary.getByteArray()[i - diff];
int firstCharIdx = startOffset + estimateMetaLen;
for (int dest = ary.getLength() - 1, src = dest - diff; src >= firstCharIdx; dest--, src--) {
ary.getByteArray()[dest] = ary.getByteArray()[src];
}
}
}
@@ -257,4 +257,37 @@ public void testTrimWithPattern() throws Exception {
assertEquals(0, expected.compareTo(result));
}

@Test
public void testStringBuilder() throws Exception {
UTF8StringBuilder builder = new UTF8StringBuilder();
GrowableArray array = new GrowableArray();
UTF8StringPointable stringPointable = new UTF8StringPointable();
String writtenString;
int startIdx;

array.append(STRING_UTF8_MIX.getByteArray(), STRING_UTF8_MIX.getStartOffset(), STRING_UTF8_MIX.getLength());
String chunk = "ABC";
String originalString = chunk.repeat(699051);

// test grow path
startIdx = array.getLength();
builder.reset(array, 2);
builder.appendString(originalString);
builder.finish();
stringPointable.set(array.getByteArray(), startIdx, array.getLength());
writtenString = stringPointable.toString();
assertEquals(originalString, writtenString);

// test shrink path
array.reset();
array.append(STRING_UTF8_MIX.getByteArray(), STRING_UTF8_MIX.getStartOffset(), STRING_UTF8_MIX.getLength());
startIdx = array.getLength();
builder.reset(array, 699051);
builder.appendString(chunk);
builder.finish();
stringPointable.set(array.getByteArray(), startIdx, array.getLength());
writtenString = stringPointable.toString();
assertEquals(chunk, writtenString);
}

}

0 comments on commit 5701408

Please sign in to comment.