urlencode nested serializer temp file names so they dont explode stuff (#15068)

Fixes a bug caused by #14919, which was just using the column name as part of a temp file name, which.. isn't very cool, my bad. Switched to use StringUtils.urlEncode so that ugly chars don't explode stuff. The modified test fails without the changes in this PR.
This commit is contained in:
Clint Wylie 2023-10-04 21:43:45 -07:00 committed by GitHub
parent 30cf76db99
commit 3afe09a19d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 7 deletions

View File

@ -99,7 +99,7 @@ public final class DictionaryIdLookup implements Closeable
// for strings because of this. if other type dictionary writers could potentially use multiple internal files // for strings because of this. if other type dictionary writers could potentially use multiple internal files
// in the future, we should transition them to using this approach as well (or build a combination smoosher and // in the future, we should transition them to using this approach as well (or build a combination smoosher and
// mapper so that we can have a mutable smoosh) // mapper so that we can have a mutable smoosh)
File stringSmoosh = FileUtils.createTempDir(name + "__stringTempSmoosh"); File stringSmoosh = FileUtils.createTempDir(StringUtils.urlEncode(name) + "__stringTempSmoosh");
final String fileName = NestedCommonFormatColumnSerializer.getInternalFileName( final String fileName = NestedCommonFormatColumnSerializer.getInternalFileName(
name, name,
NestedCommonFormatColumnSerializer.STRING_DICTIONARY_FILE_NAME NestedCommonFormatColumnSerializer.STRING_DICTIONARY_FILE_NAME
@ -135,7 +135,7 @@ public final class DictionaryIdLookup implements Closeable
public int lookupLong(@Nullable Long value) public int lookupLong(@Nullable Long value)
{ {
if (longDictionary == null) { if (longDictionary == null) {
Path longFile = makeTempFile(name + NestedCommonFormatColumnSerializer.LONG_DICTIONARY_FILE_NAME); final Path longFile = makeTempFile(name + NestedCommonFormatColumnSerializer.LONG_DICTIONARY_FILE_NAME);
longBuffer = mapWriter(longFile, longDictionaryWriter); longBuffer = mapWriter(longFile, longDictionaryWriter);
longDictionary = FixedIndexed.read(longBuffer, TypeStrategies.LONG, ByteOrder.nativeOrder(), Long.BYTES).get(); longDictionary = FixedIndexed.read(longBuffer, TypeStrategies.LONG, ByteOrder.nativeOrder(), Long.BYTES).get();
// reset position // reset position
@ -151,9 +151,14 @@ public final class DictionaryIdLookup implements Closeable
public int lookupDouble(@Nullable Double value) public int lookupDouble(@Nullable Double value)
{ {
if (doubleDictionary == null) { if (doubleDictionary == null) {
Path doubleFile = makeTempFile(name + NestedCommonFormatColumnSerializer.DOUBLE_DICTIONARY_FILE_NAME); final Path doubleFile = makeTempFile(name + NestedCommonFormatColumnSerializer.DOUBLE_DICTIONARY_FILE_NAME);
doubleBuffer = mapWriter(doubleFile, doubleDictionaryWriter); doubleBuffer = mapWriter(doubleFile, doubleDictionaryWriter);
doubleDictionary = FixedIndexed.read(doubleBuffer, TypeStrategies.DOUBLE, ByteOrder.nativeOrder(), Double.BYTES).get(); doubleDictionary = FixedIndexed.read(
doubleBuffer,
TypeStrategies.DOUBLE,
ByteOrder.nativeOrder(),
Double.BYTES
).get();
// reset position // reset position
doubleBuffer.position(0); doubleBuffer.position(0);
} }
@ -167,7 +172,7 @@ public final class DictionaryIdLookup implements Closeable
public int lookupArray(@Nullable int[] value) public int lookupArray(@Nullable int[] value)
{ {
if (arrayDictionary == null) { if (arrayDictionary == null) {
Path arrayFile = makeTempFile(name + NestedCommonFormatColumnSerializer.ARRAY_DICTIONARY_FILE_NAME); final Path arrayFile = makeTempFile(name + NestedCommonFormatColumnSerializer.ARRAY_DICTIONARY_FILE_NAME);
arrayBuffer = mapWriter(arrayFile, arrayDictionaryWriter); arrayBuffer = mapWriter(arrayFile, arrayDictionaryWriter);
arrayDictionary = FrontCodedIntArrayIndexed.read(arrayBuffer, ByteOrder.nativeOrder()).get(); arrayDictionary = FrontCodedIntArrayIndexed.read(arrayBuffer, ByteOrder.nativeOrder()).get();
// reset position // reset position
@ -239,7 +244,7 @@ public final class DictionaryIdLookup implements Closeable
private Path makeTempFile(String name) private Path makeTempFile(String name)
{ {
try { try {
return Files.createTempFile(name, ".tmp"); return Files.createTempFile(StringUtils.urlEncode(name), null);
} }
catch (IOException e) { catch (IOException e) {
throw new RuntimeException(e); throw new RuntimeException(e);

View File

@ -171,7 +171,7 @@ public class NestedDataColumnSupplierTest extends InitializedNullHandlingTest
@Before @Before
public void setup() throws IOException public void setup() throws IOException
{ {
final String fileNameBase = "test"; final String fileNameBase = "test/column";
final String arrayFileNameBase = "array"; final String arrayFileNameBase = "array";
fileMapper = smooshify(fileNameBase, tempFolder.newFolder(), data); fileMapper = smooshify(fileNameBase, tempFolder.newFolder(), data);
baseBuffer = fileMapper.mapFile(fileNameBase); baseBuffer = fileMapper.mapFile(fileNameBase);