Skip to content

Commit 1d5884d

Browse files
dekimirdain
authored andcommitted
Throw TrinoException on invalid Hive timestamp format
We were previously throwing IllegalArgumentException, which is turned into a GENERIC_INTERNAL_ERROR by QueryStateMachine. That's wrong, as the error is external.
1 parent 664f4e4 commit 1d5884d

File tree

3 files changed

+59
-19
lines changed

3 files changed

+59
-19
lines changed

lib/trino-hive-formats/src/main/java/io/trino/hive/formats/HiveFormatUtils.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import com.google.common.collect.ImmutableList;
1717
import io.trino.plugin.base.type.DecodedTimestamp;
18+
import io.trino.spi.TrinoException;
1819
import io.trino.spi.block.Block;
1920
import io.trino.spi.block.BlockBuilder;
2021
import io.trino.spi.type.DecimalConversions;
@@ -44,7 +45,7 @@
4445
import java.util.regex.Matcher;
4546
import java.util.regex.Pattern;
4647

47-
import static com.google.common.base.Preconditions.checkArgument;
48+
import static io.trino.hive.formats.HiveFormatsErrorCode.HIVE_INVALID_METADATA;
4849
import static io.trino.spi.type.DateType.DATE;
4950
import static io.trino.spi.type.Decimals.overflows;
5051
import static io.trino.spi.type.Timestamps.MILLISECONDS_PER_SECOND;
@@ -264,18 +265,15 @@ public static List<String> getTimestampFormatsSchemaProperty(Map<String, String>
264265
char c = property.charAt(position);
265266
if (c == TIMESTAMP_FORMATS_ESCAPE) {
266267
// the next character must be an escape or separator
267-
checkArgument(
268-
position + 1 < property.length(),
269-
"Invalid '%s' property value '%s': unterminated escape at end of value",
270-
TIMESTAMP_FORMATS_KEY,
271-
property);
268+
if (position + 1 >= property.length()) {
269+
throw new TrinoException(HIVE_INVALID_METADATA,
270+
"Invalid '%s' property value '%s': unterminated escape at end of value".formatted(TIMESTAMP_FORMATS_KEY, property));
271+
}
272272
char nextCharacter = property.charAt(position + 1);
273-
checkArgument(
274-
nextCharacter == TIMESTAMP_FORMATS_SEPARATOR || nextCharacter == TIMESTAMP_FORMATS_ESCAPE,
275-
"Invalid '%s' property value '%s': Illegal escaped character at %s",
276-
TIMESTAMP_FORMATS_KEY,
277-
property,
278-
position);
273+
if (nextCharacter != TIMESTAMP_FORMATS_SEPARATOR && nextCharacter != TIMESTAMP_FORMATS_ESCAPE) {
274+
throw new TrinoException(HIVE_INVALID_METADATA,
275+
"Invalid '%s' property value '%s': Illegal escaped character at %s".formatted(TIMESTAMP_FORMATS_KEY, property, position));
276+
}
279277

280278
buffer.append(nextCharacter);
281279
position++;
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package io.trino.hive.formats;
15+
16+
import io.trino.spi.ErrorCode;
17+
import io.trino.spi.ErrorCodeSupplier;
18+
import io.trino.spi.ErrorType;
19+
20+
import static io.trino.spi.ErrorType.EXTERNAL;
21+
22+
// these error codes must match the error codes in HiveErrorCode
23+
public enum HiveFormatsErrorCode
24+
implements ErrorCodeSupplier
25+
{
26+
HIVE_INVALID_METADATA(12, EXTERNAL),
27+
/**/;
28+
29+
private final ErrorCode errorCode;
30+
31+
HiveFormatsErrorCode(int code, ErrorType type)
32+
{
33+
errorCode = new ErrorCode(code + 0x0100_0000, name(), type);
34+
}
35+
36+
@Override
37+
public ErrorCode toErrorCode()
38+
{
39+
return errorCode;
40+
}
41+
}

lib/trino-hive-formats/src/test/java/io/trino/hive/formats/TestHiveFormatUtils.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
import static io.trino.hive.formats.HiveFormatUtils.TIMESTAMP_FORMATS_KEY;
2323
import static io.trino.hive.formats.HiveFormatUtils.getTimestampFormatsSchemaProperty;
2424
import static io.trino.hive.formats.HiveFormatUtils.parseHiveDate;
25+
import static io.trino.hive.formats.HiveFormatsErrorCode.HIVE_INVALID_METADATA;
26+
import static io.trino.testing.assertions.TrinoExceptionAssert.assertTrinoExceptionThrownBy;
2527
import static org.assertj.core.api.Assertions.assertThat;
26-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2728

2829
public class TestHiveFormatUtils
2930
{
@@ -39,12 +40,12 @@ public void test()
3940
@Test
4041
public void testTimestampFormatEscaping()
4142
{
42-
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() ->
43-
getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "\\")))
44-
.withMessageContaining("unterminated escape");
45-
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() ->
46-
getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "\\neither backslash nor comma")))
47-
.withMessageContaining("Illegal escaped character");
43+
assertTrinoExceptionThrownBy(() -> getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "\\")))
44+
.hasErrorCode(HIVE_INVALID_METADATA)
45+
.hasMessageContaining("unterminated escape");
46+
assertTrinoExceptionThrownBy(() -> getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "\\neither backslash nor comma")))
47+
.hasErrorCode(HIVE_INVALID_METADATA)
48+
.hasMessageContaining("Illegal escaped character");
4849
assertThat(getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "\\\\"))).isEqualTo(ImmutableList.of("\\"));
4950
assertThat(getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "xx\\\\"))).isEqualTo(ImmutableList.of("xx\\"));
5051
assertThat(getTimestampFormatsSchemaProperty(ImmutableMap.of(TIMESTAMP_FORMATS_KEY, "\\\\yy"))).isEqualTo(ImmutableList.of("\\yy"));

0 commit comments

Comments
 (0)