Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add integration JDBC tests for cursor/fetch_size feature. #208

Merged
merged 6 commits into from
Feb 2, 2023
Merged
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
134 changes: 132 additions & 2 deletions integ-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ dependencies {
testImplementation group: 'org.opensearch.test', name: 'framework', version: "${opensearch_version}"
testImplementation group: 'org.opensearch.client', name: 'opensearch-rest-high-level-client', version: "${opensearch_version}"
testImplementation group: 'org.opensearch.client', name: 'opensearch-rest-client', version: "${opensearch_version}"
testImplementation group: 'org.opensearch.driver', name: 'opensearch-sql-jdbc', version: '1.2.0.0'
testImplementation group: 'org.opensearch.driver', name: 'opensearch-sql-jdbc', version: System.getProperty("jdbcDriverVersion", '1.2.0.0')
testImplementation group: 'org.hamcrest', name: 'hamcrest', version: '2.1'
implementation group: 'org.apache.logging.log4j', name: 'log4j-core', version:'2.17.1'
testImplementation project(':opensearch-sql-plugin')
Expand Down Expand Up @@ -109,10 +109,10 @@ compileTestJava {

testClusters.all {
testDistribution = 'archive'
plugin ":opensearch-sql-plugin"
}

testClusters.integTest {
plugin ":opensearch-sql-plugin"
keystore 'plugins.query.federation.datasources.config', new File("$projectDir/src/test/resources/datasource/", 'datasources.json')
}

Expand Down Expand Up @@ -148,8 +148,119 @@ task stopPrometheus(type: KillProcessTask) {

stopPrometheus.mustRunAfter startPrometheus

task cloneJdbcDriverRepo {

Choose a reason for hiding this comment

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

Suggested change
task cloneJdbcDriverRepo {
task jdbcShadowJarFromRepo {

Choose a reason for hiding this comment

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

Or better yet, separate clone from build so that we don't need to delete/download the JDBC driver from repo each and every time.

Copy link
Author

@Yury-Fridlyand Yury-Fridlyand Jan 30, 2023

Choose a reason for hiding this comment

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

It was your requirement to run tests on in-development JDBC driver. Note, this executed in :integ-test:integDevJdbcTest gradle task only if jdbcFile parameter not given. This gradle task is not a part of build or :integ-test:integTest tasks.

Choose a reason for hiding this comment

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

Yes. I just think those two tasks could be split into separate tasks rather than having to download + build... in case the build fails.

Choose a reason for hiding this comment

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

Mostly a suggestion I think at this point.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in 0b817f4

if (file("${buildDir}/sql-jdbc").exists()) {
exec {
workingDir "${buildDir}/sql-jdbc"
commandLine 'git', 'remote', 'remove', 'origin'
}
exec {
workingDir "${buildDir}/sql-jdbc"
commandLine 'git', 'remote', 'add', 'origin', System.getProperty('jdbcRepo', 'https://github.com/opensearch-project/sql-jdbc.git')
}
exec {
workingDir "${buildDir}/sql-jdbc"
commandLine 'git', 'fetch', 'origin'
}
exec {
workingDir "${buildDir}/sql-jdbc"
commandLine 'git', 'reset', '--hard', 'origin/' + System.getProperty("jdbcBranch", 'main')
}
} else {
exec {
workingDir buildDir
// clone the sql-jdbc repo locally
commandLine 'git', 'clone', '--branch', System.getProperty("jdbcBranch", 'main'), System.getProperty('jdbcRepo', 'https://github.com/opensearch-project/sql-jdbc.git')
}
}
// TODO would fail on windows
exec {
workingDir "${buildDir}/sql-jdbc"
commandLine 'sh', 'gradlew', 'shadowJar'
}
}

task integJdbcTest(type: RestIntegTestTask) {
useJUnitPlatform()
dependsOn ':opensearch-sql-plugin:bundlePlugin'
testLogging {
events "passed", "skipped", "failed"
}
afterTest { desc, result ->
logger.quiet "${desc.className}.${desc.name}: ${result.resultType} ${(result.getEndTime() - result.getStartTime())/1000}s"
}

systemProperty 'tests.security.manager', 'false'
systemProperty('project.root', project.projectDir.absolutePath)

systemProperty "https", System.getProperty("https")
systemProperty "user", System.getProperty("user")
systemProperty "password", System.getProperty("password")

// Set default query size limit
systemProperty 'defaultQuerySizeLimit', '10000'

// Tell the test JVM if the cluster JVM is running under a debugger so that tests can use longer timeouts for
// requests. The 'doFirst' delays reading the debug setting on the cluster till execution time.
doFirst { systemProperty 'cluster.debug', getDebug() }

if (System.getProperty("test.debug") != null) {
jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005'
}

filter {
includeTestsMatching 'org.opensearch.sql.jdbc.*'
}
}

task integDevJdbcTest(type: RestIntegTestTask) {
useJUnitPlatform()
dependsOn ':opensearch-sql-plugin:bundlePlugin'
testLogging {
events "passed", "skipped", "failed"
}
afterTest { desc, result ->
logger.quiet "${desc.className}.${desc.name}: ${result.resultType} ${(result.getEndTime() - result.getStartTime())/1000}s"
}

if (System.getProperty("jdbcFile") != null) {
file("${buildDir}/sql-jdbc/build/libs").mkdirs()
copy {
from System.getProperty("jdbcFile")
into "${buildDir}/sql-jdbc/build/libs"
}
} else {
dependsOn cloneJdbcDriverRepo
}

systemProperty 'tests.security.manager', 'false'
systemProperty('project.root', project.projectDir.absolutePath)

systemProperty "https", System.getProperty("https")
systemProperty "user", System.getProperty("user")
systemProperty "password", System.getProperty("password")

// Set default query size limit
systemProperty 'defaultQuerySizeLimit', '10000'

// Tell the test JVM if the cluster JVM is running under a debugger so that tests can use longer timeouts for
// requests. The 'doFirst' delays reading the debug setting on the cluster till execution time.
doFirst { systemProperty 'cluster.debug', getDebug() }

if (System.getProperty("test.debug") != null) {
jvmArgs '-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005'
}

filter {
includeTestsMatching 'org.opensearch.sql.devJdbc.*'
}
}

// Run PPL ITs and new, legacy and comparison SQL ITs with new SQL engine enabled
integTest {
testLogging {
events "passed", "skipped", "failed"
}
dependsOn ':opensearch-sql-plugin:bundlePlugin'
dependsOn startPrometheus
finalizedBy stopPrometheus
Expand Down Expand Up @@ -192,10 +303,17 @@ integTest {

// Skip this IT because all assertions are against explain output
exclude 'org/opensearch/sql/legacy/OrderIT.class'

// Exclude JDBC related tests
exclude 'org/opensearch/sql/jdbc/**'
exclude 'org/opensearch/sql/devJdbc/**'
}


task comparisonTest(type: RestIntegTestTask) {
testLogging {
events "passed", "skipped", "failed"
}
dependsOn ':opensearch-sql-plugin:bundlePlugin'

systemProperty 'tests.security.manager', 'false'
Expand All @@ -214,6 +332,10 @@ task comparisonTest(type: RestIntegTestTask) {
exclude 'org/opensearch/sql/ppl/**/*IT.class'
exclude 'org/opensearch/sql/legacy/**/*IT.class'

// Exclude JDBC related tests
exclude 'org/opensearch/sql/jdbc/**'
exclude 'org/opensearch/sql/devJdbc/**'

// Enable logging output to console
testLogging.showStandardStreams true

Expand Down Expand Up @@ -368,6 +490,9 @@ task "${baseName}#fullRestartClusterTask"(type: StandaloneRestIntegTestTask) {

// A bwc test suite which runs all the bwc tasks combined
task bwcTestSuite(type: StandaloneRestIntegTestTask) {
testLogging {
events "passed", "skipped", "failed"
}
exclude '**/*Test*'
exclude '**/*IT*'
dependsOn tasks.named("${baseName}#mixedClusterTask")
Expand All @@ -379,6 +504,9 @@ def opensearch_tmp_dir = rootProject.file('build/private/es_tmp').absoluteFile
opensearch_tmp_dir.mkdirs()

task integTestRemote(type: RestIntegTestTask) {
testLogging {
events "passed", "skipped", "failed"
}
testClassesDirs = sourceSets.test.output.classesDirs
classpath = sourceSets.test.runtimeClasspath
systemProperty 'tests.security.manager', 'false'
Expand Down Expand Up @@ -406,4 +534,6 @@ task integTestRemote(type: RestIntegTestTask) {
exclude 'org/opensearch/sql/legacy/TermQueryExplainIT.class'
exclude 'org/opensearch/sql/legacy/QueryAnalysisIT.class'
exclude 'org/opensearch/sql/legacy/OrderIT.class'
exclude 'org/opensearch/sql/jdbc/**'
exclude 'org/opensearch/sql/devJdbc/**'
}
129 changes: 129 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/devJdbc/CursorIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.devJdbc;

import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_CALCS;
import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;
import static org.opensearch.sql.util.TestUtils.getResponseBody;

import java.io.File;
import java.net.URL;
import java.net.URLClassLoader;
import java.sql.Connection;
import java.sql.Driver;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.List;
import javax.annotation.Nullable;
import lombok.SneakyThrows;
import org.json.JSONObject;
import org.junit.AfterClass;
import org.junit.Assume;
import org.junit.BeforeClass;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.opensearch.client.Request;
import org.opensearch.client.RequestOptions;
import org.opensearch.client.Response;
import org.opensearch.sql.legacy.SQLIntegTestCase;

public class CursorIT extends SQLIntegTestCase {

private static Connection connection;
private static Driver driver;
private boolean initialized = false;

@BeforeEach
@SneakyThrows
public void init() {
if (!initialized) {

Choose a reason for hiding this comment

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

why isn't this under the @BeforeAll below?

Copy link
Author

Choose a reason for hiding this comment

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

I call initClient from init, which can't be called from a static method. @BeforeAll requires method to be static.

Choose a reason for hiding this comment

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

You could also do

if (client() == null) {
      initClient();
    }

like the other classes.

Copy link
Author

Choose a reason for hiding this comment

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

initClient doesn't work in a static context

initClient();
resetQuerySizeLimit();
loadIndex(Index.BANK);
loadIndex(Index.CALCS);
loadIndex(Index.ONLINE);
loadIndex(Index.ACCOUNT);
initialized = true;
}
}

@BeforeAll
@BeforeClass
@SneakyThrows
public static void loadDriver() {
var buildDir = String.format("%s/build/sql-jdbc/build/libs", System.getProperty("project.root"));
var driverFiles = new File(buildDir).
listFiles(pathname -> pathname.getAbsolutePath().endsWith(".jar"));

Assume.assumeTrue("Driver load failed", driverFiles != null && 1 == driverFiles.length);

URLClassLoader loader = new URLClassLoader(
new URL[] { driverFiles[0].toURI().toURL() },
ClassLoader.getSystemClassLoader()
);
driver = (Driver)Class.forName("org.opensearch.jdbc.Driver", true, loader)
.getDeclaredConstructor().newInstance();
connection = driver.connect(getConnectionString(), null);
}

@AfterAll
@AfterClass
@SneakyThrows
public static void closeConnection() {
// TODO should we close Statement and ResultSet?
if (connection != null) {
connection.close();
connection = null;
}
}

@Test
@SneakyThrows
public void dev_select_all_small_table_small_cursor() {
Statement stmt = connection.createStatement();

for (var table : List.of(TEST_INDEX_CALCS, TEST_INDEX_BANK)) {
var query = String.format("SELECT * FROM %s", table);
stmt.setFetchSize(3);
ResultSet rs = stmt.executeQuery(query);
int rows = 0;
for (; rs.next(); rows++) ;

var restResponse = executeRestQuery(query, null);
assertEquals(rows, restResponse.getInt("total"));
}
}

/**
* Use OpenSearch cluster initialized by OpenSearch Gradle task.
*/
private static String getConnectionString() {
// string like "[::1]:46751,127.0.0.1:34403"
var clusterUrls = System.getProperty("tests.rest.cluster").split(",");
return String.format("jdbc:opensearch://%s", clusterUrls[clusterUrls.length - 1]);
}

@SneakyThrows
protected JSONObject executeRestQuery(String query, @Nullable Integer fetch_size) {
Request request = new Request("POST", QUERY_API_ENDPOINT);
if (fetch_size != null) {
request.setJsonEntity(String.format("{ \"query\": \"%s\", \"fetch_size\": %d }", query, fetch_size));
} else {
request.setJsonEntity(String.format("{ \"query\": \"%s\" }", query));
}

RequestOptions.Builder restOptionsBuilder = RequestOptions.DEFAULT.toBuilder();
restOptionsBuilder.addHeader("Content-Type", "application/json");
request.setOptions(restOptionsBuilder);

Response response = client().performRequest(request);
return new JSONObject(getResponseBody(response));
}

}
Loading