zabetak commented on code in PR #6197:
URL: https://github.com/apache/hive/pull/6197#discussion_r2658004740
##########
ql/src/java/org/apache/hadoop/hive/ql/plan/TableDesc.java:
##########
@@ -137,7 +138,7 @@ public void setJobProperties(Map<String, String>
jobProperties) {
this.jobProperties = jobProperties;
}
- @Explain(displayName = "jobProperties", explainLevels = { Level.EXTENDED })
+ @Explain(displayName = ExplainTask.JOB_PROPERTIES, explainLevels = {
Level.EXTENDED })
Review Comment:
This changes seems unrelated to the PR. From a quick search it seems that
whenever we use `@Explain(displayName` we always have a plain string so for
consistency reasons I would leave it as is.
##########
ql/src/test/queries/clientpositive/jdbc_case_sensitive_postgres.q:
##########
@@ -0,0 +1,76 @@
+--! qt:database:postgres:qdb:q_test_case_sensitive.postgres.sql
+
+-- ==============================================================
+-- Test with AUTHORIZATION ENABLED
+-- ==============================================================
+
+set hive.test.authz.sstd.hs2.mode=true;
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set hive.security.authorization.enabled=true;
+
+-- Test Case-Sensitive Schema and Table
+-- This CREATE will pass, but the SELECT will fail authorization.
Review Comment:
This is a positive test what does it mean that SELECT fails authorization?
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/GenericJdbcDatabaseAccessor.java:
##########
@@ -537,12 +539,27 @@ public boolean needColumnQuote() {
return true;
}
+ /**
+ * Quotes an identifier based on the database type.
+ *
+ * @param identifier The identifier to quote
+ * @param dbType The DatabaseType enum
+ * @return A database-specific quoted identifier (e.g., "\"Country\"" or
"`Country`")
+ */
+ protected static String quoteIdentifier(String identifier, DatabaseType
dbType) {
Review Comment:
For the record, Calcite provides the following APIs:
* org.apache.calcite.sql.SqlDialect.Context#identifierQuoteString
* org.apache.calcite.sql.SqlDialect#quoteIdentifier(java.lang.String)
It may be possible to use them instead of introducing code that does very
similar things.
##########
ql/src/test/queries/clientpositive/jdbc_case_sensitive_postgres.q:
##########
@@ -0,0 +1,76 @@
+--! qt:database:postgres:qdb:q_test_case_sensitive.postgres.sql
+
+-- ==============================================================
+-- Test with AUTHORIZATION ENABLED
+-- ==============================================================
+
+set hive.test.authz.sstd.hs2.mode=true;
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set hive.security.authorization.enabled=true;
+
+-- Test Case-Sensitive Schema and Table
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE country_test (id int, name varchar(20))
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Country\""
+);
+SELECT * FROM country_test;
+
+
+-- Test Case-Sensitive Partition Column
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE cities_test (id int, name varchar(20), regionid int)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Cities\"",
+ "hive.sql.partitionColumn" = "RegionID",
Review Comment:
Why do we need quotes for schema and table but not for partition column?
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/GenericJdbcDatabaseAccessor.java:
##########
@@ -537,12 +539,27 @@ public boolean needColumnQuote() {
return true;
}
+ /**
+ * Quotes an identifier based on the database type.
+ *
+ * @param identifier The identifier to quote
+ * @param dbType The DatabaseType enum
+ * @return A database-specific quoted identifier (e.g., "\"Country\"" or
"`Country`")
+ */
+ protected static String quoteIdentifier(String identifier, DatabaseType
dbType) {
+ if (identifier == null || dbType == null || dbType.getStartQuote() ==
null) {
+ return identifier;
+ }
+ return dbType.getStartQuote() + identifier + dbType.getEndQuote();
+ }
+
private static String getQualifiedTableName(Configuration conf) {
- String tableName = conf.get(Constants.JDBC_TABLE);
+ DatabaseType dbType =
DatabaseType.from(conf.get(JdbcStorageConfig.DATABASE_TYPE.getPropertyName()));
+ String tableName =
quoteIdentifier(unescapeHiveJdbcIdentifier(conf.get(Constants.JDBC_TABLE)),
dbType);
if (tableName == null) {
return null;
}
- String schemaName = conf.get(Constants.JDBC_SCHEMA);
+ String schemaName =
quoteIdentifier(unescapeHiveJdbcIdentifier(conf.get(Constants.JDBC_SCHEMA)),
dbType);
Review Comment:
If we always quote the schema/table name then we are making all code relying
on this method case-sensitive. In other words, if previously the user had
defined `"hive.sql.table" = "Country"` then queries over this table may stop
working due to case sensitivity. Quoting everything seems to be a breaking
change.
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainTask.java:
##########
@@ -1161,15 +1135,20 @@ public JSONObject outputPlan(Object work, PrintStream
out,
}
// Try this as a map
- if (val instanceof Map) {
- // Go through the map and print out the stuff
- Map<?, ?> mp = (Map<?, ?>) val;
+ if (val instanceof Map<?, ?> mp) {
+
+ // Remove JDBC username from job properties
Review Comment:
Why are we removing the username? How is this related to the sensitivity of
identifiers?
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:
##########
@@ -5094,4 +5094,32 @@ public static String getTableOrMVSuffix(Context context,
boolean createTableOrMV
}
return suffix;
}
+
+ /**
+ * Unescape a Hive JDBC identifier, removing surrounding double quotes if
present.
+ * @param identifier the identifier to unescape
+ * @return the unescaped identifier
+ */
+ public static String unescapeHiveJdbcIdentifier(String identifier) {
+ return unescapeIdentifier(identifier, '"');
Review Comment:
The choice of `"` as the "Hive" identifier seems a bit arbitrary. Who/Where
do we define the values that are allowed inside `hive.sql.table`,
`hive.sql.schema`, etc?
What prevents a MSSQL user to write `"hive.sql.table"="[WorldData]"`. Was
this working before? Is this working now?
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/conf/JdbcStorageConfigManager.java:
##########
@@ -67,8 +67,7 @@ public static void copyConfigurationToJob(Properties props,
Map<String, String>
if (!key.equals(CONFIG_PWD) &&
!key.equals(CONFIG_PWD_KEYSTORE) &&
!key.equals(CONFIG_PWD_KEY) &&
- !key.equals(CONFIG_PWD_URI) &&
- !key.equals(CONFIG_USERNAME)
Review Comment:
Why are we changing this code? Why username is relevant to case-sensitivity
of identifiers?
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/GenericJdbcDatabaseAccessor.java:
##########
@@ -364,7 +366,7 @@ protected String addBoundaryToQuery(String tableName,
String sql, String partiti
Matcher m = fromPattern.matcher(sql);
Preconditions.checkArgument(m.matches());
- if (!tableName.equals(m.group(2).replaceAll("[`\"]", ""))) {
Review Comment:
This seems to change current behavior. We don't need to handle the single
quote character anymore?
##########
data/scripts/q_test_case_sensitive.mariadb.sql:
##########
@@ -0,0 +1,30 @@
+-- Create a case-sensitive schema (database)
+CREATE SCHEMA `WorldData`;
+
+-- Case-Sensitive Schema and Table
+CREATE TABLE `WorldData`.`Country`
+(
+ id int,
+ name varchar(20)
+);
+
+INSERT INTO `WorldData`.`Country` VALUES (1, 'India'), (2, 'USA'), (3,
'Japan'), (4, 'Germany');
+
+
+-- Case-Sensitive Partition Column
+CREATE TABLE `WorldData`.`Cities`
+(
+ id int,
+ name varchar(20),
+ `RegionID` int
Review Comment:
what happens if there is another unquoted column: `regionid int`
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/conf/DatabaseType.java:
##########
@@ -15,14 +15,68 @@
package org.apache.hive.storage.jdbc.conf;
public enum DatabaseType {
- MYSQL,
+ // Default quote is "
H2,
DB2,
DERBY,
ORACLE,
POSTGRES,
- MSSQL,
- METASTORE,
JETHRO_DATA,
- HIVE
+ MSSQL,
+
+ // Special quote cases
+ MYSQL("`"),
+ MARIADB("`"),
+ HIVE("`"),
+
+ // METASTORE will be resolved to another type
+ METASTORE(null, null);
+
+ // Keeping start and end quote separate to allow for future DBs
+ // that may have different start and end quotes
+ private final String startQuote;
+ private final String endQuote;
Review Comment:
We can consider different start/end quotes when the time comes. For now, we
don't need to complicate the code if we don't support such use-cases.
##########
ql/src/test/queries/clientpositive/jdbc_case_sensitive_postgres.q:
##########
@@ -0,0 +1,76 @@
+--! qt:database:postgres:qdb:q_test_case_sensitive.postgres.sql
+
+-- ==============================================================
+-- Test with AUTHORIZATION ENABLED
+-- ==============================================================
+
+set hive.test.authz.sstd.hs2.mode=true;
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set hive.security.authorization.enabled=true;
+
+-- Test Case-Sensitive Schema and Table
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE country_test (id int, name varchar(20))
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Country\""
+);
+SELECT * FROM country_test;
+
+
+-- Test Case-Sensitive Partition Column
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE cities_test (id int, name varchar(20), regionid int)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Cities\"",
+ "hive.sql.partitionColumn" = "RegionID",
Review Comment:
What happens if the underlying table has another column `regionid` that is
not quoted? How can we select between the two?
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/DBRecordWritable.java:
##########
@@ -64,8 +64,13 @@ public void write(PreparedStatement statement) throws
SQLException {
ParameterMetaData parameterMetaData = statement.getParameterMetaData();
for (int i = 0; i < columnValues.length; i++) {
Object value = columnValues[i];
- if ((parameterMetaData.getParameterType(i + 1) == Types.CHAR) && value
!= null && value instanceof Boolean) {
- value = ((Boolean) value).booleanValue() ? "1" : "0";
+ try {
+ if (value instanceof Boolean b &&
(parameterMetaData.getParameterType(i + 1) == Types.CHAR)) {
+ value = b ? "1" : "0";
+ }
+ } catch (SQLException e) {
Review Comment:
Catching and ignoring any kind of `SQLException` is suspicious. It gives the
impression that something is broken in the code.
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/conf/DatabaseType.java:
##########
@@ -15,14 +15,68 @@
package org.apache.hive.storage.jdbc.conf;
public enum DatabaseType {
- MYSQL,
+ // Default quote is "
H2,
DB2,
DERBY,
ORACLE,
POSTGRES,
- MSSQL,
- METASTORE,
JETHRO_DATA,
- HIVE
+ MSSQL,
+
+ // Special quote cases
+ MYSQL("`"),
+ MARIADB("`"),
+ HIVE("`"),
+
+ // METASTORE will be resolved to another type
+ METASTORE(null, null);
+
+ // Keeping start and end quote separate to allow for future DBs
+ // that may have different start and end quotes
+ private final String startQuote;
+ private final String endQuote;
+
+ DatabaseType() {
+ this("\"", "\"");
+ }
+
+ DatabaseType(String quote) {
+ this(quote, quote);
+ }
+
+ DatabaseType(String startQuote, String endQuote) {
+ this.startQuote = startQuote;
+ this.endQuote = endQuote;
+ }
+
+ /**
+ * Helper to safely get the DatabaseType from a string.
+ *
+ * @param dbType The string from configuration properties.
+ * @return The matching DatabaseType.
+ * @throws IllegalArgumentException if the dbType is null or not a valid
type.
+ */
+ public static DatabaseType from(String dbType) {
Review Comment:
Why do we need this method and not simply use valueOf?
##########
ql/src/test/queries/clientpositive/jdbc_case_sensitive_postgres.q:
##########
@@ -0,0 +1,76 @@
+--! qt:database:postgres:qdb:q_test_case_sensitive.postgres.sql
+
+-- ==============================================================
+-- Test with AUTHORIZATION ENABLED
+-- ==============================================================
+
+set hive.test.authz.sstd.hs2.mode=true;
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set hive.security.authorization.enabled=true;
+
+-- Test Case-Sensitive Schema and Table
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE country_test (id int, name varchar(20))
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Country\""
+);
+SELECT * FROM country_test;
+
+
+-- Test Case-Sensitive Partition Column
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE cities_test (id int, name varchar(20), regionid int)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Cities\"",
+ "hive.sql.partitionColumn" = "RegionID",
+ "hive.sql.numPartitions" = "2"
+);
+SELECT * FROM cities_test where regionid >= 20;
+
+
+-- Test Case-Sensitive Query Field Names
+-- (Should fail in SerDe/Iterator with Column not found)
+-- This tests the bug in JdbcSerDe and JdbcRecordIterator
+CREATE EXTERNAL TABLE geography_test (id int, description varchar(50))
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.query" = "SELECT id, \"Description\" FROM
\"WorldData\".\"Geography\""
+);
+SELECT * FROM geography_test;
+
+-- ==============================================================
+-- Test with AUTHORIZATION DISABLED
+-- ==============================================================
+
+set hive.security.authorization.enabled=false;
+
+SELECT * FROM country_test;
+
+SELECT * FROM cities_test where regionid >= 20;
+
+SELECT * FROM geography_test;
+
+
+-- Cleanup
+DROP TABLE country_test;
+DROP TABLE cities_test;
+DROP TABLE geography_test;
Review Comment:
I have the impression that the framework takes care of the cleanup so these
DROP statements are redundant.
##########
ql/src/test/queries/clientpositive/jdbc_case_sensitive_postgres.q:
##########
@@ -0,0 +1,76 @@
+--! qt:database:postgres:qdb:q_test_case_sensitive.postgres.sql
+
+-- ==============================================================
+-- Test with AUTHORIZATION ENABLED
+-- ==============================================================
+
+set hive.test.authz.sstd.hs2.mode=true;
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set hive.security.authorization.enabled=true;
+
+-- Test Case-Sensitive Schema and Table
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE country_test (id int, name varchar(20))
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
Review Comment:
I have the impression that we also support single quoted table properties so
it may be a bit more readable to use those instead of escaping double quotes:
```
'hive.sql.schema'='"WorldData"'
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/conf/DatabaseType.java:
##########
@@ -15,14 +15,68 @@
package org.apache.hive.storage.jdbc.conf;
public enum DatabaseType {
- MYSQL,
+ // Default quote is "
H2,
DB2,
DERBY,
ORACLE,
POSTGRES,
- MSSQL,
- METASTORE,
JETHRO_DATA,
- HIVE
+ MSSQL,
+
+ // Special quote cases
+ MYSQL("`"),
+ MARIADB("`"),
+ HIVE("`"),
+
+ // METASTORE will be resolved to another type
+ METASTORE(null, null);
+
+ // Keeping start and end quote separate to allow for future DBs
+ // that may have different start and end quotes
+ private final String startQuote;
+ private final String endQuote;
+
+ DatabaseType() {
+ this("\"", "\"");
+ }
+
+ DatabaseType(String quote) {
+ this(quote, quote);
+ }
+
+ DatabaseType(String startQuote, String endQuote) {
+ this.startQuote = startQuote;
+ this.endQuote = endQuote;
+ }
+
+ /**
+ * Helper to safely get the DatabaseType from a string.
+ *
+ * @param dbType The string from configuration properties.
+ * @return The matching DatabaseType.
+ * @throws IllegalArgumentException if the dbType is null or not a valid
type.
+ */
+ public static DatabaseType from(String dbType) {
+ if (dbType == null) {
+ throw new IllegalArgumentException("Database type string cannot be
null");
+ }
+ // METASTORE must be handled before valueOf
+ if (METASTORE.name().equalsIgnoreCase(dbType)) {
Review Comment:
Why does METASTORE need special handling?
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -3179,8 +3180,8 @@ private RelNode genTableLogicalPlan(String tableAlias, QB
qb) throws SemanticExc
LOG.warn("No password found for accessing {} table via JDBC",
fullyQualifiedTabName);
}
final String catalogName =
tabMetaData.getProperty(Constants.JDBC_CATALOG);
- final String schemaName =
tabMetaData.getProperty(Constants.JDBC_SCHEMA);
- final String tableName =
tabMetaData.getProperty(Constants.JDBC_TABLE);
+ final String schemaName =
unescapeHiveJdbcIdentifier(tabMetaData.getProperty(Constants.JDBC_SCHEMA));
+ final String tableName =
unescapeHiveJdbcIdentifier(tabMetaData.getProperty(Constants.JDBC_TABLE));
Review Comment:
What happens if the user defines two tables with the following props:
* `"hive.sql.table"="[WorldData]"`
* `"hive.sql.table"="WorldData"`
The underlying DBMS may also have two tables defined as such.
Aren't we risking having conflicts/ambiguity if we "unescape" the content of
the properties?
##########
ql/src/test/queries/clientpositive/jdbc_case_sensitive_postgres.q:
##########
@@ -0,0 +1,76 @@
+--! qt:database:postgres:qdb:q_test_case_sensitive.postgres.sql
+
+-- ==============================================================
+-- Test with AUTHORIZATION ENABLED
+-- ==============================================================
+
+set hive.test.authz.sstd.hs2.mode=true;
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set hive.security.authorization.enabled=true;
+
+-- Test Case-Sensitive Schema and Table
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE country_test (id int, name varchar(20))
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Country\""
+);
+SELECT * FROM country_test;
+
+
+-- Test Case-Sensitive Partition Column
+-- This CREATE will pass, but the SELECT will fail authorization.
+CREATE EXTERNAL TABLE cities_test (id int, name varchar(20), regionid int)
+STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
+TBLPROPERTIES (
+ "hive.sql.database.type" = "POSTGRES",
+ "hive.sql.jdbc.driver" = "org.postgresql.Driver",
+ "hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
+ "hive.sql.dbcp.username" =
"${system:hive.test.database.qdb.jdbc.username}",
+ "hive.sql.dbcp.password" =
"${system:hive.test.database.qdb.jdbc.password}",
+ "hive.sql.schema" = "\"WorldData\"",
+ "hive.sql.table" = "\"Cities\"",
+ "hive.sql.partitionColumn" = "RegionID",
+ "hive.sql.numPartitions" = "2"
+);
+SELECT * FROM cities_test where regionid >= 20;
+
+
+-- Test Case-Sensitive Query Field Names
+-- (Should fail in SerDe/Iterator with Column not found)
Review Comment:
If the test should fail it should be in the negative directory.
##########
jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java:
##########
@@ -101,12 +103,12 @@ public void configureInputJobProperties(TableDesc
tableDesc, Map<String, String>
@Override
public URI getURIForAuth(Table table) throws URISyntaxException {
Map<String, String> tableProperties =
HiveCustomStorageHandlerUtils.getTableProperties(table);
- DatabaseType dbType = DatabaseType.valueOf(
+ DatabaseType dbType = DatabaseType.from(
tableProperties.get(JdbcStorageConfig.DATABASE_TYPE.getPropertyName()));
- String host_url = DatabaseType.METASTORE == dbType ?
+ String hostUrl = DatabaseType.METASTORE == dbType ?
"jdbc:metastore://" : tableProperties.get(Constants.JDBC_URL);
- String table_name = tableProperties.get(Constants.JDBC_TABLE);
- return new URI(host_url+"/"+table_name);
+ String tableName =
unescapeHiveJdbcIdentifier(tableProperties.get(Constants.JDBC_TABLE));
Review Comment:
If we "unescape" some content of the property aren't we risking
conflicts/ambiguity.
For instance:
* `"hive.sql.table"="\"Country\""`
* `"hive.sql.table"="Country"`
Moreover, it seems that the current unescaping is not gonna work for other
quoting chars:
* `"hive.sql.table"="[Country]"`
* "hive.sql.table"="\`Country\`"
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
