Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
ayushtkn merged PR #5554: URL: https://github.com/apache/hive/pull/5554 -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
sonarcloud[bot] commented on PR #5554: URL: https://github.com/apache/hive/pull/5554#issuecomment-2538390706 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
linghengqian commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1881577265
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,16 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+String userName =
connection.getConnParams().getSessionVars().get(JdbcConnectionParams.AUTH_USER);
+if ((userName == null) || userName.isEmpty()) {
+ userName = JdbcConnectionParams.ANONYMOUS_USER;
+}
+return userName;
}
Review Comment:
Done.
##
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestHiveDatabaseMetaData.java:
##
@@ -18,18 +18,15 @@
package org.apache.hive.jdbc;
-import org.apache.hive.jdbc.HiveConnection;
-import org.apache.hive.jdbc.Utils;
+import com.google.common.collect.ImmutableMap;
import org.apache.hive.jdbc.Utils.JdbcConnectionParams;
+import org.junit.Before;
+import org.junit.Test;
+import java.sql.SQLException;
+import java.util.HashMap;
import java.util.LinkedHashMap;
-import java.util.Properties;
import java.util.Map;
-import java.util.HashMap;
-import java.sql.SQLException;
-
-import org.junit.Before;
-import org.junit.Test;
Review Comment:
Done.
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
ayushtkn commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1878532349
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,16 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+String userName =
connection.getConnParams().getSessionVars().get(JdbcConnectionParams.AUTH_USER);
+if ((userName == null) || userName.isEmpty()) {
+ userName = JdbcConnectionParams.ANONYMOUS_USER;
+}
+return userName;
}
Review Comment:
can we just do
```
public String getUserName() throws SQLException {
return connection.getUserName();
}
```
and make ``getUserName()`` in ``HiveConnection`` package-private (default)?
##
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestHiveDatabaseMetaData.java:
##
@@ -18,18 +18,15 @@
package org.apache.hive.jdbc;
-import org.apache.hive.jdbc.HiveConnection;
-import org.apache.hive.jdbc.Utils;
+import com.google.common.collect.ImmutableMap;
import org.apache.hive.jdbc.Utils.JdbcConnectionParams;
+import org.junit.Before;
+import org.junit.Test;
+import java.sql.SQLException;
+import java.util.HashMap;
import java.util.LinkedHashMap;
-import java.util.Properties;
import java.util.Map;
-import java.util.HashMap;
-import java.sql.SQLException;
-
-import org.junit.Before;
-import org.junit.Test;
Review Comment:
Can you just avoid changing the import order, just makes things complicated
while backporting
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
sonarcloud[bot] commented on PR #5554: URL: https://github.com/apache/hive/pull/5554#issuecomment-2502672269 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
sonarcloud[bot] commented on PR #5554: URL: https://github.com/apache/hive/pull/5554#issuecomment-2497857922 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
sonarcloud[bot] commented on PR #5554: URL: https://github.com/apache/hive/pull/5554#issuecomment-2494631459 ## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5554&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5554&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5554&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5554) -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
linghengqian commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1854153479
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,12 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+return connection.getConnParams().getSessionVars().get("user");
Review Comment:
- I added new unit tests. I guess Hive Doc does not prohibit the use of
Guava's utility classes in unit tests.
- I must admit that I found the Hive Contributor Guide on ASF Confluence to
be out of date, and I had to spend some time looking up how to start a single
unit test locally.
```shell
sdk install java 8.0.422-tem
sdk use java 8.0.422-tem
sdk install maven
mvn clean install -DskipTests -Pitests
mvn test -Dtest=TestHiveDatabaseMetaData -Dtest.output.overwrite=true -pl
itests/hive-unit -Pitests
```
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
InvisibleProgrammer commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1854054288
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,12 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+return connection.getConnParams().getSessionVars().get("user");
Review Comment:
> HiveConnection#getSessionValue(String, String) and
HiveConnection#getUserName() are both private methods. I guess what you mean is?
Yes, something like that.
> Changing the org.apache.hive.jdbc.TestJdbcDriver doesn't seem intuitive
enough, should I create a org.apache.hive.jdbc.TestHiveDatabaseMetaData class?
Excuse me, wrong copy-paste. So, there is already a test class here:
https://github.com/apache/hive/blob/master/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestHiveDatabaseMetaData.java
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
InvisibleProgrammer commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1854054288
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,12 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+return connection.getConnParams().getSessionVars().get("user");
Review Comment:
> HiveConnection#getSessionValue(String, String) and
HiveConnection#getUserName() are both private methods. I guess what you mean is?
Yes, something like that.
> Changing the org.apache.hive.jdbc.TestJdbcDriver doesn't seem intuitive
enough, should I create a org.apache.hive.jdbc.TestHiveDatabaseMetaData class?
Excuse me, wrong copy-paste. So, there is already a test class here:
https://github.com/apache/hive/blob/master/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestHiveDatabaseMetaData.java
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
linghengqian commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1854038652
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,12 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+return connection.getConnParams().getSessionVars().get("user");
Review Comment:
- `HiveConnection#getSessionValue(String, String)` and
`HiveConnection#getUserName()` are both private methods. I guess what you mean
is?
```java
public String getUserName() throws SQLException {
String username =
connection.getConnParams().getSessionVars().get(JdbcConnectionParams.AUTH_USER);
if ((username == null) || username.isEmpty()) {
username = JdbcConnectionParams.ANONYMOUS_USER;
}
return username;
}
```
- Changing the `org.apache.hive.jdbc.TestJdbcDriver` doesn't seem intuitive
enough, should I create a `org.apache.hive.jdbc.TestHiveDatabaseMetaData` class?
- By the way, Jenkins used by Hive limits a single PR to 5 hours between two
CIs, and the current PR's github tag is a bit inconsistent with the actual
situation.
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
InvisibleProgrammer commented on code in PR #5554:
URL: https://github.com/apache/hive/pull/5554#discussion_r1854012159
##
jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java:
##
@@ -744,12 +744,12 @@ public T getObject(int columnIndex, Class type)
throws SQLException {
};
}
- public String getURL() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+ public String getURL() {
+return connection.getConnectedUrl();
}
public String getUserName() throws SQLException {
-throw new SQLFeatureNotSupportedException("Method not supported");
+return connection.getConnParams().getSessionVars().get("user");
Review Comment:
I think it would be more bullet-proof to get the user name similar way as
HiveConnection does: It gets the username, or it is not provided, gives back
the anonymous:
```java
/**
* @return username from sessConfMap
*/
private String getUserName() {
return getSessionValue(JdbcConnectionParams.AUTH_USER,
JdbcConnectionParams.ANONYMOUS_USER);
}
```
--
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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-15540: Impl `DatabaseMetaData#getURL()` and `DatabaseMetaData#getUserName()` for HiveServer2 JDBC Driver [hive]
InvisibleProgrammer commented on PR #5554: URL: https://github.com/apache/hive/pull/5554#issuecomment-2493913622 I just looked into the jdbc driver recently and got surprised similarly as you. Can I ask you to add some tests around the changes? I think `TestJdbcDriver` can be a great place to do that. -- 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. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
