strongduanmu commented on code in PR #36492:
URL: https://github.com/apache/shardingsphere/pull/36492#discussion_r2328949933
##########
test/e2e/env/src/test/java/org/apache/shardingsphere/test/e2e/env/container/atomic/storage/impl/HiveContainer.java:
##########
@@ -95,13 +98,37 @@ protected Optional<String> getDefaultDatabaseName() {
@Override
protected void postStart() {
try {
+ createDatabasesFromConfiguration();
+ log.info("Databases created successfully in postStart()");
execInContainer("bash", "-c",
- "beeline -u \"jdbc:hive2://localhost:10000/default\" -e
\"CREATE DATABASE IF NOT EXISTS encrypt; CREATE DATABASE IF NOT EXISTS
expected_dataset;\"");
- System.out.println("Databases created successfully in
postStart()");
+ "if [ -f /docker-entrypoint-initdb.d/01-actual-init.sql ];
then beeline -u \"jdbc:hive2://localhost:10000/default\" -f
/docker-entrypoint-initdb.d/01-actual-init.sql; fi");
+ execInContainer("bash", "-c",
+ "if [ -f /docker-entrypoint-initdb.d/01-expected-init.sql
]; then beeline -u \"jdbc:hive2://localhost:10000/default\" -f
/docker-entrypoint-initdb.d/01-expected-init.sql; fi");
+ log.info("Initialization SQL files executed successfully");
} catch (final InterruptedException | IOException ex) {
- System.err.println("Failed to create databases in postStart(): " +
ex.getMessage());
+ log.error("Failed to execute postStart operations: {}",
ex.getMessage());
}
super.postStart();
- System.out.println("Hive container postStart completed successfully");
+ log.info("Hive container postStart completed successfully");
+ }
+
+ private void createDatabasesFromConfiguration() throws
InterruptedException, IOException {
+ Collection<String> actualDatabaseNames = getDatabaseNames();
+ Collection<String> expectedDatabaseNames = getExpectedDatabaseNames();
+ Collection<String> allDatabaseNames = new HashSet<>();
+ allDatabaseNames.addAll(actualDatabaseNames);
+ allDatabaseNames.addAll(expectedDatabaseNames);
+
Review Comment:
Please remove useless empty line.
##########
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/DataSetEnvironmentManager.java:
##########
@@ -234,6 +259,19 @@ public Void call() throws SQLException {
return null;
}
+ private DatabaseType getDatabaseType(final Connection connection)
throws SQLException {
+ try {
+ String url = connection.getMetaData().getURL();
+ return DatabaseTypeFactory.get(url);
+ } catch (final SQLFeatureNotSupportedException ex) {
+ String driverName = connection.getMetaData().getDriverName();
+ if (driverName != null &&
driverName.toLowerCase().contains("hive")) {
Review Comment:
Please put null on the left hand.
##########
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/DataSetEnvironmentManager.java:
##########
@@ -192,23 +197,43 @@ private static class InsertTask implements Callable<Void>
{
private final Collection<SQLValueGroup> sqlValueGroups;
+ private final DatabaseType databaseType;
+
@Override
public Void call() throws SQLException {
try (
Connection connection = dataSource.getConnection();
PreparedStatement preparedStatement =
connection.prepareStatement(insertSQL)) {
- for (SQLValueGroup each : sqlValueGroups) {
- setParameters(preparedStatement, each);
- preparedStatement.addBatch();
+ boolean isHive =
"Hive".equalsIgnoreCase(databaseType.getType());
+ if (isHive) {
+ for (SQLValueGroup each : sqlValueGroups) {
+ setParameters(preparedStatement, each);
+ preparedStatement.executeUpdate();
+ }
+ } else {
+ for (SQLValueGroup each : sqlValueGroups) {
+ setParameters(preparedStatement, each);
+ preparedStatement.addBatch();
+ }
+ preparedStatement.executeBatch();
}
- preparedStatement.executeBatch();
}
return null;
}
private void setParameters(final PreparedStatement preparedStatement,
final SQLValueGroup sqlValueGroup) throws SQLException {
for (SQLValue each : sqlValueGroup.getValues()) {
- preparedStatement.setObject(each.getIndex(), each.getValue());
+ Object value = each.getValue();
+ int index = each.getIndex();
+ if ("Hive".equalsIgnoreCase(databaseType.getType())) {
+ if (value instanceof Date) {
Review Comment:
Why call setString for Date type?
##########
test/e2e/env/src/test/java/org/apache/shardingsphere/test/e2e/env/container/atomic/storage/impl/HiveContainer.java:
##########
@@ -95,13 +98,37 @@ protected Optional<String> getDefaultDatabaseName() {
@Override
protected void postStart() {
try {
+ createDatabasesFromConfiguration();
+ log.info("Databases created successfully in postStart()");
execInContainer("bash", "-c",
- "beeline -u \"jdbc:hive2://localhost:10000/default\" -e
\"CREATE DATABASE IF NOT EXISTS encrypt; CREATE DATABASE IF NOT EXISTS
expected_dataset;\"");
- System.out.println("Databases created successfully in
postStart()");
+ "if [ -f /docker-entrypoint-initdb.d/01-actual-init.sql ];
then beeline -u \"jdbc:hive2://localhost:10000/default\" -f
/docker-entrypoint-initdb.d/01-actual-init.sql; fi");
+ execInContainer("bash", "-c",
+ "if [ -f /docker-entrypoint-initdb.d/01-expected-init.sql
]; then beeline -u \"jdbc:hive2://localhost:10000/default\" -f
/docker-entrypoint-initdb.d/01-expected-init.sql; fi");
+ log.info("Initialization SQL files executed successfully");
} catch (final InterruptedException | IOException ex) {
- System.err.println("Failed to create databases in postStart(): " +
ex.getMessage());
+ log.error("Failed to execute postStart operations: {}",
ex.getMessage());
}
super.postStart();
- System.out.println("Hive container postStart completed successfully");
+ log.info("Hive container postStart completed successfully");
+ }
+
+ private void createDatabasesFromConfiguration() throws
InterruptedException, IOException {
+ Collection<String> actualDatabaseNames = getDatabaseNames();
+ Collection<String> expectedDatabaseNames = getExpectedDatabaseNames();
+ Collection<String> allDatabaseNames = new HashSet<>();
+ allDatabaseNames.addAll(actualDatabaseNames);
+ allDatabaseNames.addAll(expectedDatabaseNames);
+
+ if (allDatabaseNames.isEmpty()) {
+ log.warn("No databases configured for Hive container");
+ return;
+ }
+ StringBuilder createDatabaseSQL = new StringBuilder();
+ for (String databaseName : allDatabaseNames) {
Review Comment:
Please rename databaseName to each.
##########
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/DataSetEnvironmentManager.java:
##########
@@ -234,6 +259,19 @@ public Void call() throws SQLException {
return null;
}
+ private DatabaseType getDatabaseType(final Connection connection)
throws SQLException {
+ try {
+ String url = connection.getMetaData().getURL();
+ return DatabaseTypeFactory.get(url);
+ } catch (final SQLFeatureNotSupportedException ex) {
+ String driverName = connection.getMetaData().getDriverName();
+ if (driverName != null &&
driverName.toLowerCase().contains("hive")) {
+ return
DatabaseTypeFactory.get("jdbc:hive2://localhost:10000/default");
Review Comment:
Why not just return HiveDatabaseType?
##########
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/DataSetEnvironmentManager.java:
##########
@@ -94,10 +96,13 @@ public void fillData() {
}
String insertSQL;
try (Connection connection =
dataSourceMap.get(dataNode.getDataSourceName()).getConnection()) {
- DatabaseType databaseType =
DatabaseTypeFactory.get(connection.getMetaData().getURL());
- insertSQL = generateInsertSQL(dataNode.getTableName(),
dataSetMetaData.getColumns(), databaseType);
+ String insertTableName = dataNode.getTableName();
+ if ("Hive".equalsIgnoreCase(databaseType.getType())) {
+ insertTableName = dataNode.getDataSourceName() + "." +
dataNode.getTableName();
Review Comment:
Why add `dataNode.getDataSourceName()` in insertTableName?
##########
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/env/DataSetEnvironmentManager.java:
##########
@@ -192,23 +197,43 @@ private static class InsertTask implements Callable<Void>
{
private final Collection<SQLValueGroup> sqlValueGroups;
+ private final DatabaseType databaseType;
+
@Override
public Void call() throws SQLException {
try (
Connection connection = dataSource.getConnection();
PreparedStatement preparedStatement =
connection.prepareStatement(insertSQL)) {
- for (SQLValueGroup each : sqlValueGroups) {
- setParameters(preparedStatement, each);
- preparedStatement.addBatch();
+ boolean isHive =
"Hive".equalsIgnoreCase(databaseType.getType());
+ if (isHive) {
Review Comment:
Can you use `DatabaseMetaData.supportsBatchUpdates()` to judge this logic?
##########
test/e2e/sql/src/test/java/org/apache/shardingsphere/test/e2e/sql/it/sql/dml/BaseDMLE2EIT.java:
##########
@@ -234,14 +234,23 @@ private void assertDataSet(final DataSetMetaData
expectedDataSetMetaData, final
private String generateFetchActualDataSQL(final Map<String, DataSource>
actualDataSourceMap, final DataNode dataNode, final DatabaseType databaseType)
throws SQLException {
String tableName = dataNode.getTableName();
Optional<String> primaryKeyColumnName =
DialectDatabaseAssertionMetaDataFactory.getPrimaryKeyColumnName(databaseType,
actualDataSourceMap.get(dataNode.getDataSourceName()), tableName);
- return primaryKeyColumnName.map(optional -> String.format("SELECT *
FROM %s ORDER BY %s ASC", tableName, optional)).orElseGet(() ->
String.format("SELECT * FROM %s", tableName));
+ if (primaryKeyColumnName.isPresent()) {
+ return String.format("SELECT * FROM %s ORDER BY %s ASC",
tableName, primaryKeyColumnName.get());
+ }
+ if ("Hive".equalsIgnoreCase(databaseType.getType())) {
Review Comment:
Why order by 1 for Hive?
--
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]
