Skip to content

Commit 5512ffd

Browse files
zabetakzabetak
authored
HIVE-29032: Enhance qt:database option to expose connection properties in qfiles (#5919)
1 parent f4f5133 commit 5512ffd

35 files changed

+270
-164
lines changed

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ public QTestUtil(QTestArguments testArgs) throws Exception {
248248
dispatcher.register("timezone", new QTestTimezoneHandler());
249249
dispatcher.register("authorizer", new QTestAuthorizerHandler());
250250
dispatcher.register("disabled", new QTestDisabledHandler());
251-
dispatcher.register("database", new QTestDatabaseHandler());
251+
dispatcher.register("database", new QTestDatabaseHandler(scriptsDir));
252252
dispatcher.register("queryhistory", new QTestQueryHistoryHandler());
253253

254254
this.initScript = scriptsDir + File.separator + testArgs.getInitScript();

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/externalDB/AbstractExternalDB.java

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,17 @@
2626
import java.io.IOException;
2727
import java.io.InputStreamReader;
2828
import java.io.PrintStream;
29+
import java.nio.file.Files;
30+
import java.nio.file.Path;
2931
import java.sql.Connection;
3032
import java.sql.DriverManager;
3133
import java.sql.SQLException;
3234
import java.util.ArrayList;
3335
import java.util.Arrays;
3436
import java.util.List;
37+
import java.util.Objects;
3538
import java.util.concurrent.TimeUnit;
39+
import java.util.function.Function;
3640

3741
/**
3842
* The class is in charge of connecting and populating dockerized databases for qtest.
@@ -43,7 +47,44 @@
4347
public abstract class AbstractExternalDB {
4448
protected static final Logger LOG = LoggerFactory.getLogger(AbstractExternalDB.class);
4549

46-
protected static final String dbName = "qtestDB";
50+
/**
51+
* A connection property that is accessible through system properties when a specific database is running.
52+
*/
53+
public enum ConnectionProperty {
54+
JDBC_URL("jdbc.url", AbstractExternalDB::getJdbcUrl),
55+
JDBC_USERNAME("jdbc.username", AbstractExternalDB::getRootUser),
56+
JDBC_PASSWORD("jdbc.password", AbstractExternalDB::getRootPassword),
57+
HOST("host", AbstractExternalDB::getContainerHostAddress),
58+
PORT("port", db -> String.valueOf(db.getPort()));
59+
60+
private final String suffix;
61+
private final Function<AbstractExternalDB, String> valueSupplier;
62+
ConnectionProperty(String suffix, Function<AbstractExternalDB, String> valueSupplier) {
63+
this.suffix = suffix;
64+
this.valueSupplier = valueSupplier;
65+
}
66+
67+
private String fullName(String dbName) {
68+
return "hive.test.database." + dbName + "." + suffix;
69+
}
70+
71+
public void set(AbstractExternalDB db) {
72+
String property = fullName(db.dbName);
73+
String value = System.getProperty(property);
74+
if (value == null) {
75+
System.setProperty(property, valueSupplier.apply(db));
76+
} else {
77+
throw new IllegalStateException("Connection property " + property + " is already set.");
78+
}
79+
}
80+
81+
public void clear(AbstractExternalDB db) {
82+
System.clearProperty(fullName(db.dbName));
83+
}
84+
}
85+
86+
protected String dbName = "qtestDB";
87+
private Path initScript;
4788

4889
private static final int MAX_STARTUP_WAIT = 5 * 60 * 1000;
4990

@@ -181,6 +222,8 @@ protected String getRootPassword() {
181222

182223
protected abstract String getJdbcDriver();
183224

225+
protected abstract int getPort();
226+
184227
protected abstract String getDockerImageName();
185228

186229
protected abstract String[] getDockerAdditionalArgs();
@@ -197,6 +240,49 @@ private String[] SQLLineCmdBuild(String sqlScriptFile) {
197240

198241
}
199242

243+
/**
244+
* Sets the name of the database. Must not be called after the database has been started.
245+
* @param name the name of the database
246+
*/
247+
public void setName(String name) {
248+
this.dbName = Objects.requireNonNull(name);
249+
}
250+
251+
/**
252+
* Sets the path to the initialization script.
253+
* @param initScript the path to the initialization script
254+
* @throws IllegalArgumentException if the script does not exist on the filesystem
255+
*/
256+
public void setInitScript(Path initScript) {
257+
if (Files.exists(initScript)) {
258+
this.initScript = initScript;
259+
} else {
260+
throw new IllegalArgumentException("Initialization script does not exist: " + initScript);
261+
}
262+
}
263+
264+
/**
265+
* Starts the database and performs any initialization required.
266+
*/
267+
public void start() throws Exception {
268+
launchDockerContainer();
269+
if (initScript != null) {
270+
execute(initScript.toString());
271+
}
272+
Arrays.stream(ConnectionProperty.values()).forEach(p -> p.set(this));
273+
}
274+
275+
/**
276+
* Stops the database and cleans up any resources.
277+
*
278+
* @throws IOException if an I/O error occurs
279+
* @throws InterruptedException if the thread is interrupted while waiting for the process to finish
280+
*/
281+
public void stop() throws IOException, InterruptedException {
282+
Arrays.stream(ConnectionProperty.values()).forEach(p -> p.clear(this));
283+
cleanupDockerContainer();
284+
}
285+
200286
public void execute(String script) throws IOException, SQLException, ClassNotFoundException {
201287
// Test we can connect to database
202288
Class.forName(getJdbcDriver());

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/externalDB/MSSQLServer.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public String getRootPassword() {
3131

3232
@Override
3333
public String getJdbcUrl() {
34-
return "jdbc:sqlserver://" + getContainerHostAddress() + ":1433";
34+
return "jdbc:sqlserver://" + getContainerHostAddress() + ":" + getPort();
3535
}
3636

3737
@Override
@@ -46,7 +46,13 @@ public String getDockerImageName() {
4646

4747
@Override
4848
public String[] getDockerAdditionalArgs() {
49-
return new String[] { "-p", "1433:1433", "-e", "ACCEPT_EULA=Y", "-e", "SA_PASSWORD=" + getRootPassword(), "-d" };
49+
return new String[] { "-p", getPort() + ":1433", "-e", "ACCEPT_EULA=Y", "-e", "SA_PASSWORD=" + getRootPassword(),
50+
"-d" };
51+
}
52+
53+
@Override
54+
protected int getPort() {
55+
return 1433;
5056
}
5157

5258
@Override

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/externalDB/MariaDB.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,22 @@ public String getRootUser() {
2929

3030
@Override
3131
public String getJdbcUrl() {
32-
return "jdbc:mariadb://" + getContainerHostAddress() + ":3309/" + dbName;
32+
return "jdbc:mariadb://" + getContainerHostAddress() + ":" + getPort() + "/" + dbName;
3333
}
3434

3535
public String getJdbcDriver() {
3636
return "org.mariadb.jdbc.Driver";
3737
}
3838

39+
@Override
40+
protected int getPort() {
41+
return 3309;
42+
}
43+
3944
public String getDockerImageName() { return "mariadb:11.4"; }
4045

4146
public String[] getDockerAdditionalArgs() {
42-
return new String[] {"-p", "3309:3306",
47+
return new String[] { "-p", getPort() + ":3306",
4348
"-e", "MARIADB_ROOT_PASSWORD=" + getRootPassword(),
4449
"-e", "MARIADB_DATABASE=" + dbName,
4550
"-d"

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/externalDB/MySQLExternalDB.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,22 @@ public String getRootUser() {
3535

3636
@Override
3737
public String getJdbcUrl() {
38-
return "jdbc:mysql://" + getContainerHostAddress() + ":3306/" + dbName;
38+
return "jdbc:mysql://" + getContainerHostAddress() + ":" + getPort() + "/" + dbName;
3939
}
4040

4141
public String getJdbcDriver() {
4242
return "com.mysql.jdbc.Driver";
4343
}
4444

45+
@Override
46+
protected int getPort() {
47+
return 3306;
48+
}
49+
4550
public String getDockerImageName() { return "mysql:8.4.3"; }
4651

4752
public String[] getDockerAdditionalArgs() {
48-
return new String[] {"-p", "3306:3306",
53+
return new String[] { "-p", getPort() + ":3306",
4954
"-e", "MYSQL_ROOT_PASSWORD=" + getRootPassword(),
5055
"-e", "MYSQL_DATABASE=" + dbName,
5156
"-d"

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/externalDB/Oracle.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,27 @@ public String getRootPassword() {
3030

3131
@Override
3232
public String getJdbcUrl() {
33-
return "jdbc:oracle:thin:@//" + getContainerHostAddress() + ":1521/xe";
33+
return "jdbc:oracle:thin:@//" + getContainerHostAddress() + ":" + getPort() + "/xe";
3434
}
3535

3636
@Override
3737
public String getJdbcDriver() {
3838
return "oracle.jdbc.OracleDriver";
3939
}
4040

41+
@Override
42+
protected int getPort() {
43+
return 1521;
44+
}
45+
4146
@Override
4247
protected String getDockerImageName() {
4348
return "abstractdog/oracle-xe:18.4.0-slim";
4449
}
4550

4651
@Override
4752
protected String[] getDockerAdditionalArgs() {
48-
return new String[] { "-p", "1521:1521", "-d", "-e", "ORACLE_PASSWORD=" + getRootPassword() };
53+
return new String[] { "-p", getPort() + ":1521", "-d", "-e", "ORACLE_PASSWORD=" + getRootPassword() };
4954
}
5055

5156
@Override

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/externalDB/PostgresExternalDB.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,24 @@ public PostgresExternalDB() {
2727
}
2828

2929
public String getJdbcUrl() {
30-
return "jdbc:postgresql://" + getContainerHostAddress() + ":5432/" + dbName;
30+
return "jdbc:postgresql://" + getContainerHostAddress() + ":" + getPort() + "/" + dbName;
3131
}
3232

3333
public String getJdbcDriver() {
3434
return "org.postgresql.Driver";
3535
}
3636

37+
@Override
38+
protected int getPort() {
39+
return 5432;
40+
}
41+
3742
public String getDockerImageName() {
3843
return "postgres:9.3";
3944
}
4045

4146
public String[] getDockerAdditionalArgs() {
42-
return new String[] {"-p", "5432:5432",
47+
return new String[] { "-p", getPort() + ":5432",
4348
"-e", "POSTGRES_PASSWORD=" + getRootPassword(),
4449
"-e", "POSTGRES_USER=" + getRootUser(),
4550
"-e", "POSTGRES_DB=" + dbName,

‎itests/util/src/main/java/org/apache/hadoop/hive/ql/qoption/QTestDatabaseHandler.java

Lines changed: 26 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,18 @@
2727
import org.slf4j.Logger;
2828
import org.slf4j.LoggerFactory;
2929

30-
import java.nio.file.Files;
31-
import java.nio.file.Path;
3230
import java.nio.file.Paths;
31+
import java.util.ArrayList;
3332
import java.util.Arrays;
34-
import java.util.EnumMap;
35-
import java.util.Map;
33+
import java.util.List;
3634

3735
/**
3836
* An option handler for spinning (resp. stopping) databases before (resp. after) running a test.
3937
*
40-
* Syntax: qt:database:DatabaseType:path_to_init_script
41-
*
42-
* The database type ({@link DatabaseType}) is obligatory argument. The initialization script can be omitted.
38+
* Syntax: qt:database:DatabaseType:databaseName:path_to_init_script
4339
*
40+
* The database type ({@link DatabaseType}) and the database name are obligatory arguments. The initialization script can be omitted.
41+
* The database name is mainly used to distinguish between different databases when they are accessed via a system {@link AbstractExternalDB.ConnectionProperty}.
4442
* Current limitations:
4543
* <ol>
4644
* <li>Only one test/file per run</li>
@@ -82,52 +80,48 @@ AbstractExternalDB create() {
8280
abstract AbstractExternalDB create();
8381
}
8482

85-
private final Map<DatabaseType, String> databaseToScript = new EnumMap<>(DatabaseType.class);
83+
private final String scriptsDir;
84+
private final List<AbstractExternalDB> databases = new ArrayList<>();
85+
86+
public QTestDatabaseHandler(final String scriptDirectory) {
87+
this.scriptsDir = scriptDirectory;
88+
}
8689

8790
@Override
8891
public void processArguments(String arguments) {
8992
String[] args = arguments.split(":");
90-
if (args.length == 0) {
91-
throw new IllegalArgumentException("No arguments provided");
92-
}
93-
if (args.length > 2) {
93+
if (args.length < 2 || args.length > 3) {
9494
throw new IllegalArgumentException(
95-
"Too many arguments. Expected {dbtype:script}. Found: " + Arrays.toString(args));
95+
"Wrong number of arguments. Expected {dbtype:dbname:script?}. Found: " + Arrays.toString(args));
9696
}
9797
DatabaseType dbType = DatabaseType.valueOf(args[0].toUpperCase());
98-
String initScript = args.length == 2 ? args[1] : "";
99-
if (databaseToScript.containsKey(dbType)) {
100-
throw new IllegalArgumentException(dbType + " database is already defined in this file.");
98+
String dbName = args[1];
99+
String initScript = args.length == 3 ? args[2] : null;
100+
AbstractExternalDB db = dbType.create();
101+
db.setName(dbName);
102+
if (initScript != null && !initScript.isEmpty()) {
103+
db.setInitScript(Paths.get(scriptsDir, initScript));
101104
}
102-
databaseToScript.put(dbType, initScript);
105+
databases.add(db);
103106
}
104107

105108
@Override
106109
public void beforeTest(QTestUtil qt) throws Exception {
107-
for (Map.Entry<DatabaseType, String> dbEntry : databaseToScript.entrySet()) {
108-
String scriptsDir = QTestUtil.getScriptsDir(qt.getConf());
109-
Path dbScript = Paths.get(scriptsDir, dbEntry.getValue());
110-
AbstractExternalDB db = dbEntry.getKey().create();
111-
db.launchDockerContainer();
112-
if (Files.exists(dbScript)) {
113-
db.execute(dbScript.toString());
114-
} else {
115-
LOG.warn("Initialization script {} not found", dbScript);
116-
}
110+
for (AbstractExternalDB db : databases) {
111+
db.start();
117112
}
118113
}
119114

120115
@Override
121116
public void afterTest(QTestUtil qt) throws Exception {
122-
for (Map.Entry<DatabaseType, String> dbEntry : databaseToScript.entrySet()) {
123-
AbstractExternalDB db = dbEntry.getKey().create();
117+
for (AbstractExternalDB db : databases) {
124118
try {
125-
db.cleanupDockerContainer();
119+
db.stop();
126120
} catch (Exception e) {
127-
LOG.error("Failed to cleanup database {}", dbEntry.getKey(), e);
121+
LOG.error("Failed to cleanup database {}", db, e);
128122
}
129123
}
130-
databaseToScript.clear();
124+
databases.clear();
131125
}
132126

133127
}

‎ql/src/test/queries/clientpositive/cbo_jdbc_joincost.q

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
--!qt:database:mysql:q_test_author_book_tables.sql
1+
--!qt:database:mysql:qdb:q_test_author_book_tables.sql
22
CREATE EXTERNAL TABLE author
33
(
44
id int,
@@ -9,9 +9,9 @@ STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
99
TBLPROPERTIES (
1010
"hive.sql.database.type" = "MYSQL",
1111
"hive.sql.jdbc.driver" = "com.mysql.jdbc.Driver",
12-
"hive.sql.jdbc.url" = "jdbc:mysql://localhost:3306/qtestDB",
13-
"hive.sql.dbcp.username" = "root",
14-
"hive.sql.dbcp.password" = "qtestpassword",
12+
"hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
13+
"hive.sql.dbcp.username" = "${system:hive.test.database.qdb.jdbc.username}",
14+
"hive.sql.dbcp.password" = "${system:hive.test.database.qdb.jdbc.password}",
1515
"hive.sql.table" = "author"
1616
);
1717

@@ -25,9 +25,9 @@ STORED BY 'org.apache.hive.storage.jdbc.JdbcStorageHandler'
2525
TBLPROPERTIES (
2626
"hive.sql.database.type" = "MYSQL",
2727
"hive.sql.jdbc.driver" = "com.mysql.jdbc.Driver",
28-
"hive.sql.jdbc.url" = "jdbc:mysql://localhost:3306/qtestDB",
29-
"hive.sql.dbcp.username" = "root",
30-
"hive.sql.dbcp.password" = "qtestpassword",
28+
"hive.sql.jdbc.url" = "${system:hive.test.database.qdb.jdbc.url}",
29+
"hive.sql.dbcp.username" = "${system:hive.test.database.qdb.jdbc.username}",
30+
"hive.sql.dbcp.password" = "${system:hive.test.database.qdb.jdbc.password}",
3131
"hive.sql.table" = "book"
3232
);
3333

0 commit comments

Comments
 (0)