test(bigquery-jdbc): update tests to run in TPC - #14014
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors integration tests in the BigQuery JDBC driver by replacing public dataset queries with dynamically generated data using UNNEST(GENERATE_ARRAY(...)) and introducing a centralized validateStatement helper. While this reduces external dependencies, several critical issues were identified in the review. Specifically, converting queries to Standard SQL in tests configured for Legacy SQL (BIG_QUERY dialect) will cause syntax errors, and removing table references in location-restriction tests will prevent expected exceptions from being thrown. Additionally, an unaliased column in testSmallSelectAndVerifyResults may lead to result retrieval failures.
| BigQueryJdbcException ex = | ||
| assertThrows(BigQueryJdbcException.class, () -> statement.executeQuery(query)); | ||
| assertThrows(BigQueryJdbcException.class, () -> ITBase.validateStatement(statement, 180)); |
There was a problem hiding this comment.
In testIncorrectLocation, the test expects a BigQueryJdbcException to be thrown due to a location mismatch (connecting to europe-west3 but querying a US dataset).
However, the query has been changed to ITBase.validateStatement(statement, 180), which executes SELECT * FROM UNNEST(GENERATE_ARRAY(1, 180)). Since this query does not reference any tables, it does not have a location restriction and will execute successfully in europe-west3 without throwing any exception. This will cause the test to fail.
To fix this, you should revert to a query that references a table in a different region (such as the US). Since this test is already tagged with @Tag("disable_tpc"), it is safe to keep the original query referencing the public dataset.
References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| String connection_uri = ITNightlyBigQueryTest.connection_uri; | ||
| String selectLegacyQuery = | ||
| "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 20000000;"; | ||
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 20000000))"; |
There was a problem hiding this comment.
In testNonEnabledUseLegacySQLThrowsSyntaxError, the test expects a syntax error because it attempts to run a Legacy SQL query when Legacy SQL is not enabled.
By changing the query to "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 20000000))", which is valid Standard SQL, the query will execute successfully (or fail due to other reasons like resource limits, but not a syntax error). This will cause the test to fail.
You should revert this query to the original Legacy SQL query using square brackets.
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 20000000))"; | |
| String selectLegacyQuery = "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 20000000;"; |
References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| String legacyJoinQuery = | ||
| "SELECT\n" | ||
| + " repo_name\n" | ||
| + "FROM\n" | ||
| + " [bigquery-public-data.github_repos.commits],\n" | ||
| + " [bigquery-public-data.github_repos.sample_commits] LIMIT 10"; | ||
| + "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 10))" | ||
| + "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 10))"; |
There was a problem hiding this comment.
The updated query in testValidLegacySQLStatement is syntactically invalid in any SQL dialect:
SELECT
repo_name
FROM
SELECT * FROM UNNEST(GENERATE_ARRAY(1, 10))SELECT * FROM UNNEST(GENERATE_ARRAY(1, 10))Additionally, this test is configured with QueryDialect=BIG_QUERY (Legacy SQL), but the new query uses Standard SQL constructs (UNNEST, GENERATE_ARRAY). This will fail with a syntax error, causing the test to fail.
Since this test is already tagged with @Tag("disable_tpc"), you should revert it to the original valid Legacy SQL query.
| String legacyJoinQuery = | |
| "SELECT\n" | |
| + " repo_name\n" | |
| + "FROM\n" | |
| + " [bigquery-public-data.github_repos.commits],\n" | |
| + " [bigquery-public-data.github_repos.sample_commits] LIMIT 10"; | |
| + "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 10))" | |
| + "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 10))"; | |
| String legacyJoinQuery = | |
| "SELECT\n" | |
| + " repo_name\n" | |
| + "FROM\n" | |
| + " [bigquery-public-data.github_repos.commits],\n" | |
| + " [bigquery-public-data.github_repos.sample_commits] LIMIT 10"; |
References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| + "LargeResultDataset=INTEGRATION_TESTS;"; | ||
| String selectLegacyQuery = | ||
| "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 200;"; | ||
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200))"; |
There was a problem hiding this comment.
In testValidDestinationTableSavesQueriesWithLegacySQL, the connection is configured with QueryDialect=BIG_QUERY (Legacy SQL), but the query has been changed to "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200))", which is Standard SQL.
This will fail with a syntax error under Legacy SQL. Since this test is already tagged with @Tag("disable_tpc"), you should revert it to the original Legacy SQL query.
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200))"; | |
| String selectLegacyQuery = "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 200;"; |
References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| String selectLegacyQuery = | ||
| "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 250000;"; | ||
| "SELECT GENERATE_DATE_ARRAY('1900-01-01', '2000-01-01', INTERVAL 1 DAY) LIMIT 200"; |
There was a problem hiding this comment.
In testUseLegacySQLWithLargeResultsNotAllowedQueries, the connection is configured with QueryDialect=BIG_QUERY (Legacy SQL), but the query has been changed to "SELECT GENERATE_DATE_ARRAY('1900-01-01', '2000-01-01', INTERVAL 1 DAY) LIMIT 200", which is Standard SQL.
This will fail with a syntax error under Legacy SQL. Since this test is already tagged with @Tag("disable_tpc"), you should revert it to the original Legacy SQL query.
"SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 250000;";References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| String selectLegacyQuery = | ||
| "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 250000;"; | ||
| "SELECT GENERATE_DATE_ARRAY('1000-01-01', '2000-01-01', INTERVAL 1 DAY) LIMIT 250000"; |
There was a problem hiding this comment.
In testUseLegacySQLWithLargeResultsAllowedWithNoDestinationTableDefaults, the connection is configured with QueryDialect=BIG_QUERY (Legacy SQL), but the query has been changed to "SELECT GENERATE_DATE_ARRAY('1000-01-01', '2000-01-01', INTERVAL 1 DAY) LIMIT 250000", which is Standard SQL.
This will fail with a syntax error under Legacy SQL. Since this test is already tagged with @Tag("disable_tpc"), you should revert it to the original Legacy SQL query.
"SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 250000;";References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| + "LargeResultTable=FakeTable;"; | ||
| String selectLegacyQuery = | ||
| "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 200;"; | ||
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200))"; |
There was a problem hiding this comment.
In testDestinationTableWithMissingDestinationDatasetDefaults, the connection is configured with QueryDialect=BIG_QUERY (Legacy SQL), but the query has been changed to "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200))", which is Standard SQL.
This will fail with a syntax error under Legacy SQL. Since this test is already tagged with @Tag("disable_tpc"), you should revert it to the original Legacy SQL query.
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200))"; | |
| String selectLegacyQuery = "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 200;"; |
References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| + "EnableHighThroughputAPI=1;"; | ||
| String selectLegacyQuery = | ||
| "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 200000;"; | ||
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200000))"; |
There was a problem hiding this comment.
In testHTAPIWithValidDestinationTableSavesQueriesWithLegacy, the query has been changed to "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200000))", which is Standard SQL.
If this test is meant to run in Legacy SQL mode (as indicated by the test name WithLegacy and the original query using Legacy SQL brackets [...]), this new query will fail with a syntax error. Please verify if this test is indeed using Legacy SQL and, if so, revert it to the original Legacy SQL query.
| String selectLegacyQuery = "SELECT * FROM UNNEST(GENERATE_ARRAY(1, 200000))"; | |
| String selectLegacyQuery = "SELECT * FROM [bigquery-public-data.deepmind_alphafold.metadata] LIMIT 200000;"; |
References
- For integration tests, avoid refactoring existing tests to use isolated or unique datasets if they historically used a shared dataset and currently pass sequentially. Defer migration to unique datasets until parallel test execution is enabled.
| String query = | ||
| "SELECT word FROM `bigquery-public-data.samples.shakespeare` WHERE" | ||
| + " word LIKE 'X%' LIMIT 10"; | ||
| "SELECT CONCAT('X', cast(word as STRING)) FROM UNNEST(GENERATE_ARRAY(1, 10)) AS word LIMIT 10"; |
There was a problem hiding this comment.
In testSmallSelectAndVerifyResults, the query has been changed to use UNNEST(GENERATE_ARRAY(1, 10)) AS word.
Because the select expression CONCAT('X', cast(word as STRING)) is not aliased, the resulting column in the ResultSet will have an autogenerated name (e.g., _f0 or f0_) instead of "word". If the test attempts to retrieve the value by column name (e.g., resultSet.getString("word")), it will throw a SQLException.
To ensure compatibility, you should alias the select expression as word.
| "SELECT CONCAT('X', cast(word as STRING)) FROM UNNEST(GENERATE_ARRAY(1, 10)) AS word LIMIT 10"; | |
| "SELECT CONCAT('X', cast(word as STRING)) AS word FROM UNNEST(GENERATE_ARRAY(1, 10)) AS word LIMIT 10"; |
:is not allowed in SQL