Skip to content

feat: Add support for Explain feature #1852

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.

Sign up for GitHub

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

Merged
merged 51 commits into from
May 19, 2022
Merged

feat: Add support for Explain feature #1852

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
1ea42dd
Added the code for handling the explain
gauravsnj Apr 21, 2022
8c7a5b0
Update ConnectionStatementExecutorImpl.java
gauravsnj Apr 21, 2022
177f07f
Added tests
gauravsnj Apr 25, 2022
242e351
Update PG_ClientSideStatements.json
gauravsnj Apr 25, 2022
053dd92
resolved the comments
gauravsnj Apr 25, 2022
0677368
Update google-cloud-spanner/src/main/resources/com/google/cloud/spann…
gauravsnj Apr 25, 2022
87069ed
Update ConnectionStatementExecutorTest.java
gauravsnj Apr 25, 2022
436775b
Merge remote-tracking branch 'origin/explain-feature' into explain-fe…
gauravsnj Apr 25, 2022
5047608
Update PG_ClientSideStatements.json
gauravsnj Apr 25, 2022
f09c6e3
Update PG_ClientSideStatements.json
gauravsnj Apr 25, 2022
411ca77
Formatted the files for ci lint
gauravsnj Apr 25, 2022
f663865
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
gauravsnj Apr 26, 2022
4b7127d
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
gauravsnj Apr 26, 2022
e3614be
resolved some comments
gauravsnj Apr 26, 2022
e125c42
formatted the code
gauravsnj Apr 26, 2022
af79b7b
added some code
gauravsnj Apr 27, 2022
7a1937a
added support for "explain (format ) foo" kind of statements
gauravsnj Apr 28, 2022
d0c6abf
resolved some comments
gauravsnj Apr 28, 2022
9f084ba
Update ConnectionStatementExecutorImpl.java
gauravsnj Apr 28, 2022
0f76847
fixed a small bug
gauravsnj Apr 29, 2022
daddd3e
Added the code for formatting query plan for export
gauravsnj May 6, 2022
e38e917
Update ConnectionStatementExecutorImpl.java
gauravsnj May 6, 2022
60ad94e
Update ConnectionStatementExecutorImpl.java
gauravsnj May 6, 2022
e2813ab
Update ConnectionStatementExecutorImpl.java
gauravsnj May 6, 2022
32fc9de
Update ConnectionStatementExecutorImpl.java
gauravsnj May 6, 2022
e57da64
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
gauravsnj May 6, 2022
b4101ad
Changed assertThat to assertEquals
gauravsnj May 6, 2022
c59e964
Merge remote-tracking branch 'origin/explain-feature' into explain-fe…
gauravsnj May 6, 2022
a767158
removed unnecessary lines
gauravsnj May 6, 2022
1a8fd85
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
gauravsnj May 6, 2022
50808f1
Changed assertThat to assertEquals
gauravsnj May 6, 2022
1f4aa32
Update ConnectionStatementExecutorImpl.java
gauravsnj May 6, 2022
0610cae
Update ConnectionStatementExecutorImpl.java
gauravsnj May 6, 2022
faaa7cd
Added tests
gauravsnj May 12, 2022
dc9dab2
format
gauravsnj May 12, 2022
4177cdd
Update PartitionedDmlTransaction.java
gauravsnj May 12, 2022
105d8a5
generated sql script
gauravsnj May 12, 2022
6162363
resolved comments
gauravsnj May 13, 2022
d58acc0
resolved comments
gauravsnj May 13, 2022
638ae76
Update PG_ClientSideStatements.json
gauravsnj May 13, 2022
07e521c
Merge branch 'main' into explain-feature
gauravsnj May 13, 2022
dd9e83e
Update PG_ClientSideStatements.json
gauravsnj May 13, 2022
55f1736
Update PG_ClientSideStatements.json
gauravsnj May 13, 2022
2d7649f
Create ITExplainTest.java
gauravsnj May 16, 2022
40b3f89
added Integration tests
gauravsnj May 16, 2022
c272165
reformatted
gauravsnj May 17, 2022
10f06e8
changed region
gauravsnj May 17, 2022
8695453
Revert "changed region"
gauravsnj May 17, 2022
0b66c53
Update ITExplainTest.java
gauravsnj May 17, 2022
a08c293
Update ITExplainTest.java
gauravsnj May 17, 2022
9996f1c
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 17, 2022
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
Prev Previous commit
Next Next commit
resolved comments
  • Loading branch information
@gauravsnj
gauravsnj committed May 13, 2022
commit d58acc020fcbed48af90a4a3b79f9cc1869cf7e2
22 changes: 11 additions & 11 deletions ...er/src/main/java/com/google/cloud/spanner/connection/ConnectionStatementExecutorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ private String processExecutionStats(PlanNode planNode) {
return executionStats.toString();
}

private StatementResult getStatementResultFromQueryPlan(QueryPlan queryPlan, boolean isAnalyse) {
private StatementResult getStatementResultFromQueryPlan(QueryPlan queryPlan, boolean isAnalyze) {
ArrayList<Struct> list = new ArrayList<>();
for (PlanNode planNode : queryPlan.getPlanNodesList()) {
String planNodeDescription = planNode.getDisplayName();
Expand All @@ -503,19 +503,19 @@ private StatementResult getStatementResultFromQueryPlan(QueryPlan queryPlan, boo
planNodeDescription += " : " + planNode.getShortRepresentation().getDescription();
}

if (isAnalyse && !planNode.getExecutionStats().toString().equals("")) {
if (isAnalyze && !planNode.getExecutionStats().toString().equals("")) {
executionStats = processExecutionStats(planNode);
}
Struct.Builder builder = Struct.newBuilder().set("QUERY PLAN").to(planNodeDescription);

if (isAnalyse) {
if (isAnalyze) {
builder.set("EXECUTION STATS").to(executionStats);
}
list.add(builder.build());
}

ResultSet resultSet;
if (isAnalyse) {
if (isAnalyze) {
resultSet =
ResultSets.forRows(
Type.struct(
Expand Down Expand Up @@ -583,11 +583,11 @@ public StatementResult statementExplain(String sql) {
String.format("Missing closing parenthesis in the query: %s", sql));
}
String options[] = sql.substring(1, index).split("\\s*,\\s*");
boolean isAnalyse = false, startAfterIndex = false;
boolean isAnalyze = false, startAfterIndex = false;
for (String option : options) {
String optionExpression[] = option.trim().split("\\s+");
if (optionExpression.length >= 3) {
isAnalyse = false;
isAnalyze = false;
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct. If we encounter a string that contains more than 2 words, we will assume that the options expression did not contain ANALYZE and that it for the rest was valid. From what I can see in the PostgreSQL documentation, there should be no option expression that contains more than 2 words.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there are 3 strings inside parenthesis like "explain (foo bar foo, analyse true) select * from table", it will send "(foo bar foo, analyse true) select * from table" as the query which will eventually cause exception. Its just the error message will not be appropriate (it will be something like "(foo bar foo, analyse true) select * from table" is not a query).

Writing code for a solution which distinguishes the error message will be a bit tricky. Because, for the cases like "explain (select * from table)", it will be difficult to identify if the string "(select * from table)" is some option with 3 strings or a valid query string.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds reasonable to me.

} else if (ClientSideStatementExplainExecutor.EXPLAIN_OPTIONS.contains(
optionExpression[0].toLowerCase())) {
Expand All @@ -596,27 +596,27 @@ public StatementResult statementExplain(String sql) {
String.format("%s is not implemented yet", optionExpression[0]));
} else if (optionExpression[0].equalsIgnoreCase("analyse")
|| optionExpression[0].equalsIgnoreCase("analyze")) {
isAnalyse = true;
isAnalyze = true;
} else {
isAnalyse = false;
isAnalyze = false;
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also here: I think that an unknown/invalid option expression should cause a failure, not be assumed to mean 'analyze is not in the options expression'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there will be an unknown option like "explain (foo) select * from table", then it will send "(foo) select * from table" as the query, which will eventually cause exception. Its just the error message will not be appropriate (it will be something like "(foo) select * from table" is not a query).

Writing code for a solution which distinguishes the error message will be a bit tricky. Because, for the cases like "explain (select * from table)", it will be difficult to identify if the string "select" is an unknown option or a part of some query.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point.

}

if (optionExpression.length == 2) {
if (optionExpression[1].equalsIgnoreCase("false")
|| optionExpression[1].equalsIgnoreCase("0")
|| optionExpression[1].equalsIgnoreCase("off")) {
isAnalyse = false;
isAnalyze = false;
startAfterIndex = true;
} else if (!(optionExpression[1].equalsIgnoreCase("true")
|| optionExpression[1].equalsIgnoreCase("1")
|| optionExpression[1].equalsIgnoreCase("on"))) {
isAnalyse = false;
isAnalyze = false;
break;
}
}
}
if (isAnalyse) {
if (isAnalyze) {
String newSql = removeParenthesisAndTrim(sql.substring(index + 1));
return executeStatement(newSql, QueryAnalyzeMode.PROFILE);
} else if (startAfterIndex) {
Expand Down
27 changes: 21 additions & 6 deletions ...panner/src/test/java/com/google/cloud/spanner/connection/ExplainCommandConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,38 @@ public void testConvert() {
assertEquals(null, explainCommandConverter.convert("explain"));

assertEquals(
"analyse select * from table1",
explainCommandConverter.convert("explain analyse select * from table1"));
"analyze select * from table1",
explainCommandConverter.convert("explain analyze select * from table1"));
assertEquals(
"analyze \tselect * \t from table1",
explainCommandConverter.convert("explain \t analyze \tselect * \t from table1"));
assertEquals(
"analyse \n select * \t from table1",
explainCommandConverter.convert("explain \n analyse \n select * \t from table1"));
"analyze \n select * \t from table1",
explainCommandConverter.convert("explain \n analyze \n select * \t from table1"));
assertEquals(
"ANALYZE \n select * \t from table1",
explainCommandConverter.convert("EXPLAIN \n ANALYZE \n select * \t from table1"));
assertEquals(
"aNALyzE \n select * \t from table1",
explainCommandConverter.convert("ExPLaiN \n aNALyzE \n select * \t from table1"));
assertEquals(
"analyse \t select * \t from table1",
explainCommandConverter.convert("explain \n analyse \t select * \t from table1"));
"analyze \t select * \t from table1",
explainCommandConverter.convert("explain \n analyze \t select * \t from table1"));
assertEquals("analyze foo", explainCommandConverter.convert("explain analyze foo"));
assertEquals("analyze", explainCommandConverter.convert("explain analyze"));
assertEquals(
"(analyze \t select * \t from table1",
explainCommandConverter.convert("explain \n (analyze \t select * \t from table1"));
assertEquals(
"(analyze \t select * \t from table1)",
explainCommandConverter.convert("explain \n (analyze \t select * \t from table1)"));

assertEquals(
"analyse select * from table1",
explainCommandConverter.convert("explain analyse select * from table1"));
assertEquals(
"analyse \tselect * \t from table1",
explainCommandConverter.convert("explain \t analyse \tselect * \t from table1"));
assertEquals("analyse foo", explainCommandConverter.convert("explain analyse foo"));
assertEquals("analyse", explainCommandConverter.convert("explain analyse"));
}
Expand Down
69 changes: 37 additions & 32 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ExplainTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.google.cloud.spanner.ResultSet;
import com.google.cloud.spanner.SpannerException;
import com.google.cloud.spanner.Statement;
import com.google.protobuf.Descriptors.DescriptorValidationException;
import com.google.protobuf.Struct;
import com.google.protobuf.Value;
import com.google.spanner.v1.ExecuteSqlRequest;
Expand All @@ -52,7 +51,7 @@ public class ExplainTest extends AbstractMockServerTest {
Statement.of("SELECT * FROM SomeTable ORDER BY Value");

@BeforeClass
public static void setupAnalyzeResults() throws DescriptorValidationException {
public static void setupAnalyzeResults() {
mockSpanner.putStatementResult(
MockSpannerServiceImpl.StatementResult.detectDialectResult(Dialect.POSTGRESQL));
Struct metadata =
Expand Down Expand Up @@ -145,10 +144,10 @@ private void testExplain(String statement) {

private void testExplainAnalyze(String statement) {
mockSpanner.clearRequests();
final Statement explainAnalyseStatement = Statement.of(statement);
final Statement explainAnalyzeStatement = Statement.of(statement);
try (Connection connection = createConnection()) {

try (ResultSet resultSet = connection.execute(explainAnalyseStatement).getResultSet()) {
try (ResultSet resultSet = connection.execute(explainAnalyzeStatement).getResultSet()) {
int count = 0;

while (resultSet.next()) {
Expand Down Expand Up @@ -206,88 +205,94 @@ public void testValidExplain() {
}

@Test
public void testValidExplainWithFalseAnalyse() {
String statement = " explain (analyse false) " + EXPLAIN_STATEMENT_QUERY;
public void testValidExplainWithFalseAnalyze() {
String statement = " explain (analyze false) " + EXPLAIN_STATEMENT_QUERY;
testExplain(statement);

statement = " explain (analyse FALSE) " + EXPLAIN_STATEMENT_QUERY + " ";
statement = " explain (analyze FALSE) " + EXPLAIN_STATEMENT_QUERY + " ";
testExplain(statement);

statement = " explain (analyse fAlsE) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " explain (analyze fAlsE) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplain(statement);

statement = " explain (analyse 0) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " explain (analyze 0) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplain(statement);

statement = " explain (analyse off) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " explain (analyze off) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplain(statement);

statement =
" explain (analyse false, analyze true, analyse false, analyze false) "
" explain (analyze false, analyze true, analyze false, analyze false) "
+ EXPLAIN_STATEMENT_QUERY;
testExplain(statement);

statement =
" explain ( analyse off , analyse true , analyse 0 ) ( "
" explain ( analyze off , analyze true , analyze 0 ) ( "
+ EXPLAIN_STATEMENT_QUERY
+ " ) ";
testExplain(statement);

statement =
" explain ( analyse off , analyse 0 , analyse 0 ) ( "
" explain ( analyze off , analyze 0 , analyze 0 ) ( "
+ EXPLAIN_STATEMENT_QUERY
+ " ) ";
testExplain(statement);

statement =
" explain ( analyse off , analyse, analyse 0 , analyse false ) ( "
" explain ( analyze off , analyze, analyze 0 , analyze false ) ( "
+ EXPLAIN_STATEMENT_QUERY
+ " ) ";
testExplain(statement);
}

@Test
public void testValidExplainAnalyse() {
String statement = "Explain analyse " + EXPLAIN_STATEMENT_QUERY;
public void testValidExplainAnalyze() {
String statement = "Explain analyze " + EXPLAIN_STATEMENT_QUERY;
testExplainAnalyze(statement);

statement = "explain analyze " + EXPLAIN_STATEMENT_QUERY;
testExplainAnalyze(statement);

statement = "explain analyse " + EXPLAIN_STATEMENT_QUERY;
statement = "explain analyze " + EXPLAIN_STATEMENT_QUERY;
testExplainAnalyze(statement);

statement = "explain analyse (" + EXPLAIN_STATEMENT_QUERY + ") ";
statement = "explain analyze (" + EXPLAIN_STATEMENT_QUERY + ") ";
testExplainAnalyze(statement);

statement = " explain ( analyse true ) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " explain ( analyze true ) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplainAnalyze(statement);

statement = " ExpLAin( analyse 1 ) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " ExpLAin( analyze 1 ) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplainAnalyze(statement);

statement = " ExpLAin( analyse On ) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " ExpLAin( analyze On ) ( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplainAnalyze(statement);

statement = " EXPLAIN(analyse)( " + EXPLAIN_STATEMENT_QUERY + " ) ";
statement = " EXPLAIN(analyze)( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplainAnalyze(statement);

statement =
" EXPLAIN(analyse , analyse false , analyse 1)( " + EXPLAIN_STATEMENT_QUERY + " ) ";
" EXPLAIN(analyze , analyze false , analyze 1)( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplainAnalyze(statement);

statement =
" EXPLAIN(analyse , analyse false , analyse )( " + EXPLAIN_STATEMENT_QUERY + " ) ";
" EXPLAIN(analyze , aNAlyzE false , analyze )( " + EXPLAIN_STATEMENT_QUERY + " ) ";
testExplainAnalyze(statement);

statement =
" EXPLAIN(analyse off , analyse false , analyse )( "
" EXPLAIN(analyze off , analyze false , AnalYZE )( "
+ EXPLAIN_STATEMENT_QUERY
+ " ) ";
testExplainAnalyze(statement);

statement =
" EXPLAIN(analyse \n off , analyse false , analyse )( "
" EXPLAIN(analyze \n off , analyze false , analyze )( "
+ EXPLAIN_STATEMENT_QUERY
+ " \t ) ";
testExplainAnalyze(statement);

statement =
" EXPLAIN(analyse \n off , analyze false , analyse )( "
+ EXPLAIN_STATEMENT_QUERY
+ " \t ) ";
testExplainAnalyze(statement);
Expand All @@ -302,22 +307,22 @@ public void testInvalidExplain() {
String statement2 = " explain foo " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement2));

String statement3 = " explain analyse analyse " + EXPLAIN_STATEMENT_QUERY;
String statement3 = " explain analyze analyze " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement3));

String statement4 = " explain analyse true " + EXPLAIN_STATEMENT_QUERY;
String statement4 = " explain analyze true " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement4));

String statement5 = " explain (analyse true , verbose ) " + EXPLAIN_STATEMENT_QUERY;
String statement5 = " explain (analyze true , verbose ) " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement5));

String statement6 = " explain (analyse hello) " + EXPLAIN_STATEMENT_QUERY;
String statement6 = " explain (analyze hello) " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement6));

String statement7 = " explain (analyse true , verbose , costs ) " + EXPLAIN_STATEMENT_QUERY;
String statement7 = " explain (analyze true , verbose , costs ) " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement7));

String statement8 = " explain (analyse true , verbose , costs " + EXPLAIN_STATEMENT_QUERY;
String statement8 = " explain (analyze true , verbose , costs " + EXPLAIN_STATEMENT_QUERY;
assertThrows(SpannerException.class, () -> testExplain(statement8));
}
}