HunterL commented on a change in pull request #1328:
URL: https://github.com/apache/hive/pull/1328#discussion_r462373165
##########
File path:
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
##########
@@ -22,15 +22,19 @@
import org.apache.hadoop.hive.serde2.typeinfo.PrimitiveTypeInfo;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoUtils;
+import org.apache.hive.storage.jdbc.exception.HiveJdbcDatabaseAccessException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
import java.sql.SQLDataException;
import java.sql.SQLException;
import java.sql.Types;
+import java.util.Arrays;
Review comment:
I don't believe this import is used
##########
File path:
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
##########
@@ -58,10 +62,29 @@ public JdbcRecordIterator(Connection conn,
PreparedStatement ps, ResultSet rs, C
if (conf.get(Constants.JDBC_TABLE) != null &&
conf.get(Constants.JDBC_QUERY) != null) {
fieldNamesProperty =
Preconditions.checkNotNull(conf.get(Constants.JDBC_QUERY_FIELD_NAMES));
fieldTypesProperty =
Preconditions.checkNotNull(conf.get(Constants.JDBC_QUERY_FIELD_TYPES));
- } else {
- fieldNamesProperty =
Preconditions.checkNotNull(conf.get(serdeConstants.LIST_COLUMNS));
+ }
+ else {
+ try {
+ if (conf.get(Constants.JDBC_QUERY) == null) {
Review comment:
Whitespace at the end of this if is still off a bit, should be just one
space after `)`
##########
File path:
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
##########
@@ -58,10 +62,29 @@ public JdbcRecordIterator(Connection conn,
PreparedStatement ps, ResultSet rs, C
if (conf.get(Constants.JDBC_TABLE) != null &&
conf.get(Constants.JDBC_QUERY) != null) {
fieldNamesProperty =
Preconditions.checkNotNull(conf.get(Constants.JDBC_QUERY_FIELD_NAMES));
fieldTypesProperty =
Preconditions.checkNotNull(conf.get(Constants.JDBC_QUERY_FIELD_TYPES));
- } else {
- fieldNamesProperty =
Preconditions.checkNotNull(conf.get(serdeConstants.LIST_COLUMNS));
+ }
+ else {
Review comment:
Nitpicking here again sorry, lets move the else a line up `} else {`
Also regarding else without the `{ ... }` being valid, you are correct it is
valid java. Stylistically however using the brackets tends to be more readable
and seems to be the dominant style in this project. :+1:
##########
File path:
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
##########
@@ -58,10 +62,29 @@ public JdbcRecordIterator(Connection conn,
PreparedStatement ps, ResultSet rs, C
if (conf.get(Constants.JDBC_TABLE) != null &&
conf.get(Constants.JDBC_QUERY) != null) {
fieldNamesProperty =
Preconditions.checkNotNull(conf.get(Constants.JDBC_QUERY_FIELD_NAMES));
fieldTypesProperty =
Preconditions.checkNotNull(conf.get(Constants.JDBC_QUERY_FIELD_TYPES));
- } else {
- fieldNamesProperty =
Preconditions.checkNotNull(conf.get(serdeConstants.LIST_COLUMNS));
+ }
+ else {
+ try {
+ if (conf.get(Constants.JDBC_QUERY) == null) {
+ ResultSetMetaData metadata = rs.getMetaData();
+ int numColumns = metadata.getColumnCount();
+ List<String> columnNames = new ArrayList<String>(numColumns);
+ for (int i = 0; i < numColumns; i++) {
+ columnNames.add(metadata.getColumnName(i + 1));
+ }
+ fieldNamesProperty = String.join(",",columnNames);
+ }
+ else {
Review comment:
Here as well lets move the else up to line 76, `} else {`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
