Skip to content

Commit

Permalink
urlencode nested serializer temp file names so they dont explode stuff (
Browse files Browse the repository at this point in the history
apache#15068)

Fixes a bug caused by apache#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.
  • Loading branch information
clintropolis authored and ycp2 committed Nov 17, 2023
1 parent 7466179 commit 753bad8
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public int lookupString(@Nullable String value)
// 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
// 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(
name,
NestedCommonFormatColumnSerializer.STRING_DICTIONARY_FILE_NAME
Expand Down Expand Up @@ -135,7 +135,7 @@ public int lookupString(@Nullable String value)
public int lookupLong(@Nullable Long value)
{
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);
longDictionary = FixedIndexed.read(longBuffer, TypeStrategies.LONG, ByteOrder.nativeOrder(), Long.BYTES).get();
// reset position
Expand All @@ -151,9 +151,14 @@ public int lookupLong(@Nullable Long value)
public int lookupDouble(@Nullable Double value)
{
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);
doubleDictionary = FixedIndexed.read(doubleBuffer, TypeStrategies.DOUBLE, ByteOrder.nativeOrder(), Double.BYTES).get();
doubleDictionary = FixedIndexed.read(
doubleBuffer,
TypeStrategies.DOUBLE,
ByteOrder.nativeOrder(),
Double.BYTES
).get();
// reset position
doubleBuffer.position(0);
}
Expand All @@ -167,7 +172,7 @@ public int lookupDouble(@Nullable Double value)
public int lookupArray(@Nullable int[] value)
{
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);
arrayDictionary = FrontCodedIntArrayIndexed.read(arrayBuffer, ByteOrder.nativeOrder()).get();
// reset position
Expand Down Expand Up @@ -239,7 +244,7 @@ private int arrayOffset()
private Path makeTempFile(String name)
{
try {
return Files.createTempFile(name, ".tmp");
return Files.createTempFile(StringUtils.urlEncode(name), null);
}
catch (IOException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public static void staticSetup()
@Before
public void setup() throws IOException
{
final String fileNameBase = "test";
final String fileNameBase = "test/column";
final String arrayFileNameBase = "array";
fileMapper = smooshify(fileNameBase, tempFolder.newFolder(), data);
baseBuffer = fileMapper.mapFile(fileNameBase);
Expand Down

0 comments on commit 753bad8

Please sign in to comment.