StructuredType part 1. #1557
StructuredType part 1. #1557
Conversation
sfc-gh-pfus
force-pushed
the
structuredType
branch
6 times, most recently
from
751e21e to
60bc8d6
Compare
sfc-gh-pfus
force-pushed
the
structuredType
branch
from
60bc8d6 to
750cee2
Compare
sfc-gh-pfus
changed the base branch from
master
to
SNOW-956803-structured-type
| import java.io.Reader; | ||
| import java.math.BigDecimal; | ||
| import java.net.URL; | ||
| import java.sql.*; |
There was a problem hiding this comment.
let's not use wildcard imports
|
|
||
| @Override | ||
| public byte readByte() throws SQLException { | ||
| return 0; |
There was a problem hiding this comment.
do we want to add some todo comments for default values?
| import java.sql.SQLException; | ||
| import java.sql.Time; | ||
| import java.sql.Timestamp; | ||
| import java.sql.*; |
There was a problem hiding this comment.
let's not use wildcard imports
| JsonNode jsonNode = OBJECT_MAPPER.readTree((String) obj); | ||
| return new JsonSqlInput(jsonNode); | ||
| } catch (JsonProcessingException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
should we throw own exception?
| @@ -31,7 +31,8 @@ | |||
|
|
|||
| /** Arrow result set implementation */ | |||
| public class SFArrowResultSet extends SFBaseResultSet implements DataConversionContext { | |||
| static final SFLogger logger = SFLoggerFactory.getLogger(SFArrowResultSet.class); | |||
| private static final SFLogger logger = SFLoggerFactory.getLogger(SFArrowResultSet.class); | |||
| private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getObjectMapper(); | |||
There was a problem hiding this comment.
private static final field for logger has different convention than OBJECT_MAPPER - we should keep and use one of them
| JsonNode jsonNode = OBJECT_MAPPER.readTree(input); | ||
| return new JsonSqlInput(jsonNode); | ||
| } catch (JsonProcessingException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
should we throw own exception here?
| public class SnowflakeObjectTypeFactories { | ||
| private static final Map<Class<?>, Supplier<SQLData>> factories = new HashMap<>(); | ||
|
|
||
| public static synchronized void register(Class<?> type, Supplier<SQLData> factory) { |
There was a problem hiding this comment.
do we need to have synchronized method here or can we use ConcurrentHashMap instead?
| import java.util.Map; | ||
| import java.util.TimeZone; | ||
| import java.sql.Date; | ||
| import java.util.*; |
There was a problem hiding this comment.
let's avoid wildcard imports
| try { | ||
| return type.newInstance(); | ||
| } catch (InstantiationException | IllegalAccessException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
should we throw own exception?
| package net.snowflake.client.util; | ||
|
|
||
| public class Predicates { | ||
| public static void notNull(Object o, String msg) { |
There was a problem hiding this comment.
cannot we use java built-in method? https://docs.oracle.com/javase/8/docs/api/java/util/Objects.html#requireNonNull-T-java.lang.String-
# Conflicts: # src/main/java/net/snowflake/client/core/SFJsonResultSet.java
sfc-gh-pfus
force-pushed
the
structuredType
branch
from
779430a to
1ae0a2b
Compare
sfc-gh-pmotacki
force-pushed
the
structuredType
branch
from
1ae0a2b to
1b01f3b
Compare
sfc-gh-pmotacki
force-pushed
the
structuredType
branch
from
53d140d to
6281099
Compare
|
SonarQube Quality Gate
|
Overview
SNOW-XXXXX
External contributors - please answer these questions before submitting a pull request. Thanks!
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #NNNN
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Please write a short description of how your code change solves the related issue.
Pre-review checklist