Conversation
There was a problem hiding this comment.
Could it really be the same exception instance?
There was a problem hiding this comment.
Yes, I know. I think I can call it a Presto convention.
kokosing
force-pushed
the
origin/master/139_jdbc_fail
branch
from
26edf94 to
8ef2620
Compare
kokosing
force-pushed
the
origin/master/139_jdbc_fail
branch
from
8ef2620 to
cac6b91
Compare
| UnsupportedTypeHandlingStrategy unsupportedTypeHandlingStrategy = getUnsupportedTypeHandlingStrategy(session); | ||
| switch (unsupportedTypeHandlingStrategy) { | ||
| case IGNORE: | ||
| break; |
| case IGNORE: | ||
| break; | ||
| case FAIL: | ||
| throw new PrestoException(JDBC_ERROR, "Unsupported data type for column: " + columnName); |
| @Override | ||
| public List<PropertyMetadata<?>> getSessionProperties() | ||
| { | ||
| ImmutableList.Builder builder = ImmutableList.<PropertyMetadata<?>>builder(); |
| return unsupportedTypeHandlingStrategy; | ||
| } | ||
|
|
||
| @Config("unsupported-type.handling-strategy") |
There was a problem hiding this comment.
unsupported-type.handling-strategy=FAIL is not nearly as clear as fail-on-unsupported-types=true.
I understand the enum is a preparation for an option to map unsupported columns to varchar (see also #186)
I think the option name needs to be revisited.
| private String passwordCredentialName; | ||
| private boolean caseInsensitiveNameMatching; | ||
| private Duration caseInsensitiveNameMatchingCacheTtl = new Duration(1, MINUTES); | ||
| private UnsupportedTypeHandlingStrategy unsupportedTypeHandlingStrategy = FAIL; |
There was a problem hiding this comment.
Do we have already enough data points to determine what the default behavior should actually be?
| @@ -63,6 +63,7 @@ | |||
| import static com.google.common.collect.ImmutableMap.toImmutableMap; | |||
There was a problem hiding this comment.
query should fail only when user actually tries to read data from such column
|
I propose a different new behavior:
Regarding default behavior, there are different use-cases for
Since current behavior is more appropriate in certain use-cases, I would not change current default behavior. |
|
What about conversion @findepi What do you think? |
|
Seems reasonable to me. |
|
Replaced with #1182 |
4 participants
Fail on unknown JDBC type