Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions data/scripts/q_test_case_sensitive_country_table.mariadb.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-- country and `Country` are distinct tables.
CREATE SCHEMA bob;

CREATE TABLE bob.country
(
id int,
name varchar(20)
);
insert into bob.country values (1, 'India');
insert into bob.country values (2, 'Russia');
insert into bob.country values (3, 'USA');

CREATE TABLE bob.`Country`
(
id int,
name varchar(20)
);
insert into bob.`Country` values (10, 'Italy');
insert into bob.`Country` values (11, 'Greece');


24 changes: 24 additions & 0 deletions data/scripts/q_test_case_sensitive_country_table.mssql.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
-- A case-sensitive collation makes country and [Country] distinct tables.
CREATE DATABASE worldcs COLLATE Latin1_General_CS_AS;
USE worldcs;

CREATE SCHEMA bob;

CREATE TABLE bob.country
(
id int,
name varchar(20)
);
insert into bob.country values (1, 'India');
insert into bob.country values (2, 'Russia');
insert into bob.country values (3, 'USA');

CREATE TABLE bob.[Country]
(
id int,
name varchar(20)
);
insert into bob.[Country] values (10, 'Italy');
insert into bob.[Country] values (11, 'Greece');


19 changes: 19 additions & 0 deletions data/scripts/q_test_case_sensitive_country_table.mysql.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- country and `Country` are distinct tables.
CREATE TABLE country
(
id int,
name varchar(20)
);
insert into country values (1, 'India');
insert into country values (2, 'Russia');
insert into country values (3, 'USA');

CREATE TABLE `Country`
(
id int,
name varchar(20)
);
insert into `Country` values (10, 'Italy');
insert into `Country` values (11, 'Greece');


25 changes: 25 additions & 0 deletions data/scripts/q_test_case_sensitive_country_table.oracle.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-- country (unquoted, folded to COUNTRY) and "Country" (quoted, case-sensitive) are distinct tables.
ALTER SESSION SET CONTAINER = XEPDB1;

CREATE USER bob IDENTIFIED BY bobpass;
ALTER USER bob QUOTA UNLIMITED ON users;
GRANT CREATE SESSION TO bob;

CREATE TABLE bob.country
(
id int,
name varchar(20)
);
insert into bob.country values (1, 'India');
insert into bob.country values (2, 'Russia');
insert into bob.country values (3, 'USA');

CREATE TABLE bob."Country"
(
id int,
name varchar(20)
);
insert into bob."Country" values (10, 'Italy');
insert into bob."Country" values (11, 'Greece');


21 changes: 21 additions & 0 deletions data/scripts/q_test_case_sensitive_country_table.postgres.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-- country (unquoted, lowercase) and "Country" (quoted, case-sensitive) are distinct tables.
CREATE SCHEMA bob;

CREATE TABLE bob.country
(
id int,
name varchar(20)
);
insert into bob.country values (1, 'India');
insert into bob.country values (2, 'Russia');
insert into bob.country values (3, 'USA');

CREATE TABLE bob."Country"
(
id int,
name varchar(20)
);
insert into bob."Country" values (10, 'Italy');
insert into bob."Country" values (11, 'Greece');


Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.hive.storage.jdbc;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.conf.Constants;
import org.apache.hadoop.hive.metastore.HiveMetaHook;
import org.apache.hadoop.hive.metastore.api.Table;
Expand All @@ -41,13 +42,17 @@

import java.io.IOException;
import java.lang.IllegalArgumentException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.net.URI;
import java.net.URISyntaxException;

import static org.apache.hadoop.hive.ql.exec.Utilities.unquoteJdbcIdentifier;

public class JdbcStorageHandler implements HiveStorageHandler {

private static final Logger LOGGER = LoggerFactory.getLogger(JdbcStorageHandler.class);
Expand Down Expand Up @@ -107,10 +112,26 @@ public URI getURIForAuth(Table table) throws URISyntaxException {
Map<String, String> tableProperties = HiveCustomStorageHandlerUtils.getTableProperties(table);
DatabaseType dbType = DatabaseType.valueOf(
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 tableIdentifier = tableProperties.get(Constants.JDBC_TABLE);
if (tableIdentifier == null) {
throw new URISyntaxException(hostUrl, "Missing required table property: " + Constants.JDBC_TABLE);
}
// Encode only the table name to keep URI construction valid; the URI is not used in the
// JDBC URL, but as an identifier stored in the HMS.
String tableName = encodeIdentifierForAuth(tableIdentifier);
return buildAuthorizationUri(hostUrl, tableName);
}

private static String encodeIdentifierForAuth(String identifier) {
String physical = unquoteJdbcIdentifier(identifier);
return URLEncoder.encode(physical, StandardCharsets.UTF_8);
}

private static URI buildAuthorizationUri(String hostUrl, String tableName) throws URISyntaxException {
String separator = hostUrl.endsWith(Path.SEPARATOR) ? "" : Path.SEPARATOR;
return new URI(hostUrl + separator + tableName);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.hive.storage.jdbc;

import org.apache.hadoop.hive.metastore.api.SerDeInfo;
import org.apache.hadoop.hive.metastore.api.StorageDescriptor;
import org.apache.hadoop.hive.metastore.api.Table;
import org.junit.Test;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.Map;

import static org.junit.Assert.assertEquals;

/**
* Unit tests for {@link JdbcStorageHandler#getURIForAuth(Table)}.
*/
public class TestJdbcStorageHandlerAuthUri {

private static Table tableWith(Map<String, String> params) {
Table table = new Table();
table.setParameters(params);
StorageDescriptor sd = new StorageDescriptor();
sd.setSerdeInfo(new SerDeInfo("serde", "serde.lib", new HashMap<>()));
table.setSd(sd);
return table;
}

private URI authUri(String jdbcUrl, String schema, String table) throws URISyntaxException {
Map<String, String> params = new HashMap<>();
params.put("hive.sql.database.type", "POSTGRES");
params.put("hive.sql.jdbc.url", jdbcUrl);
if (schema != null) {
params.put("hive.sql.schema", schema);
}
if (table != null) {
params.put("hive.sql.table", table);
}
return new JdbcStorageHandler().getURIForAuth(tableWith(params));
}

@Test
public void testUnquotedTableUnchanged() throws Exception {
URI uri = authUri("jdbc:postgresql://host:5432/db", null, "country");
assertEquals("jdbc:postgresql://host:5432/db/country", uri.toString());
}

@Test
public void testQuotedTableStripsQuotesPreservingCase() throws Exception {
URI uri = authUri("jdbc:postgresql://host:5432/db", null, "\"Country\"");
// The physical identifier keeps its original case, and the surrounding quotes are stripped.
assertEquals("jdbc:postgresql://host:5432/db/Country", uri.toString());
}

@Test
public void testQuotedTableWithSchemaOnlyEncodesTable() throws Exception {
URI uri = authUri("jdbc:postgresql://host:5432/db", "\"World\"", "\"Country\"");
assertEquals("jdbc:postgresql://host:5432/db/Country", uri.toString());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the JDBC URL contain the schema? I've found a Stackoverflow question. There is a param currentSchema for Postgres JDBC, and search_path (or searchpath?) for non-JDBC URLs.

Changing the URL for this case might be out-of-scope for HIVE-29308.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this seems out of scope for this PR, but this can be done in a follow up.

}

@Test
public void testMissingTablePropertyFailsFast() {
try {
authUri("jdbc:postgresql://host:5432/db", null, null);
org.junit.Assert.fail("Expected URISyntaxException");
} catch (URISyntaxException e) {
assertEquals("Missing required table property: hive.sql.table", e.getReason());
}
}

@Test
public void testTableWithSpecialCharactersIsUriSafe() throws Exception {
// A quoted identifier may legally contain characters that are illegal in a raw URI; they must be encoded.
URI uri = authUri("jdbc:postgresql://host:5432/db", null, "\"Odd/Name\"");
// '/' -> %2F ; the resulting string is a valid URI.
assertEquals("jdbc:postgresql://host:5432/db/Odd%2FName", uri.toString());
}
}



30 changes: 30 additions & 0 deletions ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -5151,4 +5151,34 @@ public static void setTableCreateTime(Configuration conf, Table table) {
public static int getTableCreateTime(Configuration conf, String tableName) {
return conf.getInt(String.format("%s.%s", tableName, CREATE_TIME), 0);
}

/**
* Returns the physical (unquoted) form of a JDBC identifier supplied through table properties such as
* {@code hive.sql.table} or {@code hive.sql.schema}, for use in contexts that need the identifier exactly as
* stored in the remote catalog (JDBC metadata lookups, Calcite resolution, authorization URIs).
*
* <p>Recognises ANSI/Oracle/Postgres double quotes ({@code "id"}), MySQL/MariaDB back-ticks ({@code `id`}) and
* SQL Server brackets ({@code [id]}). Unquoted identifiers are returned unchanged.
*/
public static String unquoteJdbcIdentifier(String identifier) {
if (identifier == null || identifier.length() < 2) {
return identifier;
}
char start = identifier.charAt(0);
char end = identifier.charAt(identifier.length() - 1);
final char closing;
if (start == '"' || start == '`') {
closing = start;
} else if (start == '[') {
closing = ']';
} else {
return identifier;
}
if (end != closing) {
return identifier;
}
String inner = identifier.substring(1, identifier.length() - 1);
// A use of the closing char inside the table name is escaped by doubling it.
return inner.replace(String.valueOf(closing) + closing, String.valueOf(closing));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3056,9 +3056,9 @@ private RelNode genTableLogicalPlan(String tableAlias, QB qb) throws SemanticExc
pswd = null;
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 catalogName = Utilities.unquoteJdbcIdentifier(tabMetaData.getProperty(Constants.JDBC_CATALOG));
final String schemaName = Utilities.unquoteJdbcIdentifier(tabMetaData.getProperty(Constants.JDBC_SCHEMA));
final String tableName = Utilities.unquoteJdbcIdentifier(tabMetaData.getProperty(Constants.JDBC_TABLE));

DataSource ds = JdbcSchema.dataSource(url, driver, user, pswd);
SqlDialect jdbcDialect = JdbcSchema.createDialect(SqlDialectFactoryImpl.INSTANCE, ds);
Expand Down
44 changes: 44 additions & 0 deletions ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.apache.hadoop.hive.ql.exec.Utilities.getFileExtension;
Expand Down Expand Up @@ -902,4 +903,47 @@ private Set<String> getResultPathes(List<Path> foundManifestFiles) {
}
return resultPathes;
}

@Test
public void testUnquoteJdbcIdentifierNullAndShortInputsUnchanged() {
assertNull(Utilities.unquoteJdbcIdentifier(null));
assertEquals("", Utilities.unquoteJdbcIdentifier(""));
// A single character cannot be a matched quote pair.
assertEquals("\"", Utilities.unquoteJdbcIdentifier("\""));
}

@Test
public void testUnquoteJdbcIdentifierUnquotedInputUnchanged() {
// Unquoted identifiers keep the current (case-insensitive) behaviour.
assertEquals("Country", Utilities.unquoteJdbcIdentifier("Country"));
assertEquals("country", Utilities.unquoteJdbcIdentifier("country"));
assertEquals("COUNTRY", Utilities.unquoteJdbcIdentifier("COUNTRY"));
}

@Test
public void testUnquoteJdbcIdentifierStripsSupportedQuotePairs() {
// ANSI / Oracle / Postgres double quotes.
assertEquals("Country", Utilities.unquoteJdbcIdentifier("\"Country\""));
// MySQL / MariaDB back-ticks.
assertEquals("Country", Utilities.unquoteJdbcIdentifier("`Country`"));
// SQL Server brackets.
assertEquals("Country", Utilities.unquoteJdbcIdentifier("[Country]"));
// Schema-style identifiers with mixed case are preserved after unquoting.
assertEquals("WorldData", Utilities.unquoteJdbcIdentifier("\"WorldData\""));
}

@Test
public void testUnquoteJdbcIdentifierUnescapesDoubledQuotes() {
assertEquals("a\"b", Utilities.unquoteJdbcIdentifier("\"a\"\"b\""));
assertEquals("a`b", Utilities.unquoteJdbcIdentifier("`a``b`"));
assertEquals("a]b", Utilities.unquoteJdbcIdentifier("[a]]b]"));
}

@Test
public void testUnquoteJdbcIdentifierUnbalancedQuotesUnchanged() {
assertEquals("\"Country", Utilities.unquoteJdbcIdentifier("\"Country"));
assertEquals("Country\"", Utilities.unquoteJdbcIdentifier("Country\""));
assertEquals("[Country", Utilities.unquoteJdbcIdentifier("[Country"));
assertEquals("\"Country`", Utilities.unquoteJdbcIdentifier("\"Country`"));
}
}
Loading
Loading